[DebugInfo] Store optional DIFile::Source as pointer

getCanonicalMDString() also returns a nullptr for empty strings, which
tripped over the getSource() method. Solve the ambiguity of no source
versus an optional containing a nullptr by simply storing a pointer.

Differential Revision: https://reviews.llvm.org/D138658
This commit is contained in:
Jonas Hahnfeld 2022-12-01 14:14:43 +01:00
parent 61318fa5c7
commit c9cb4fc761
8 changed files with 60 additions and 31 deletions

View File

@ -598,11 +598,12 @@ public:
private:
std::optional<ChecksumInfo<MDString *>> Checksum;
std::optional<MDString *> Source;
/// An optional source. A nullptr means none.
MDString *Source;
DIFile(LLVMContext &C, StorageType Storage,
std::optional<ChecksumInfo<MDString *>> CS,
std::optional<MDString *> Src, ArrayRef<Metadata *> Ops);
std::optional<ChecksumInfo<MDString *>> CS, MDString *Src,
ArrayRef<Metadata *> Ops);
~DIFile() = default;
static DIFile *getImpl(LLVMContext &Context, StringRef Filename,
@ -615,15 +616,13 @@ private:
MDChecksum.emplace(CS->Kind, getCanonicalMDString(Context, CS->Value));
return getImpl(Context, getCanonicalMDString(Context, Filename),
getCanonicalMDString(Context, Directory), MDChecksum,
Source ? std::optional<MDString *>(
getCanonicalMDString(Context, *Source))
: std::nullopt,
Storage, ShouldCreate);
Source ? MDString::get(Context, *Source) : nullptr, Storage,
ShouldCreate);
}
static DIFile *getImpl(LLVMContext &Context, MDString *Filename,
MDString *Directory,
std::optional<ChecksumInfo<MDString *>> CS,
std::optional<MDString *> Source, StorageType Storage,
MDString *Source, StorageType Storage,
bool ShouldCreate = true);
TempDIFile cloneImpl() const {
@ -640,7 +639,7 @@ public:
DEFINE_MDNODE_GET(DIFile,
(MDString * Filename, MDString *Directory,
std::optional<ChecksumInfo<MDString *>> CS = std::nullopt,
std::optional<MDString *> Source = std::nullopt),
MDString *Source = nullptr),
(Filename, Directory, CS, Source))
TempDIFile clone() const { return cloneImpl(); }
@ -654,7 +653,7 @@ public:
return StringRefChecksum;
}
std::optional<StringRef> getSource() const {
return Source ? std::optional<StringRef>((*Source)->getString())
return Source ? std::optional<StringRef>(Source->getString())
: std::nullopt;
}
@ -663,7 +662,7 @@ public:
std::optional<ChecksumInfo<MDString *>> getRawChecksum() const {
return Checksum;
}
std::optional<MDString *> getRawSource() const { return Source; }
MDString *getRawSource() const { return Source; }
static StringRef getChecksumKindAsString(ChecksumKind CSKind);
static std::optional<ChecksumKind> getChecksumKind(StringRef CSKindStr);

View File

@ -4996,11 +4996,11 @@ bool LLParser::parseDIFile(MDNode *&Result, bool IsDistinct) {
else if (checksumkind.Seen || checksum.Seen)
return Lex.Error("'checksumkind' and 'checksum' must be provided together");
std::optional<MDString *> OptSource;
MDString *Source = nullptr;
if (source.Seen)
OptSource = source.Val;
Result = GET_OR_DISTINCT(DIFile, (Context, filename.Val, directory.Val,
OptChecksum, OptSource));
Source = source.Val;
Result = GET_OR_DISTINCT(
DIFile, (Context, filename.Val, directory.Val, OptChecksum, Source));
return false;
}

View File

@ -1621,11 +1621,10 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
Checksum.emplace(static_cast<DIFile::ChecksumKind>(Record[3]),
getMDString(Record[4]));
MetadataList.assignValue(
GET_OR_DISTINCT(DIFile, (Context, getMDString(Record[1]),
getMDString(Record[2]), Checksum,
Record.size() > 5 ? std::optional<MDString *>(
getMDString(Record[5]))
: std::nullopt)),
GET_OR_DISTINCT(DIFile,
(Context, getMDString(Record[1]),
getMDString(Record[2]), Checksum,
Record.size() > 5 ? getMDString(Record[5]) : nullptr)),
NextMetadataNo);
NextMetadataNo++;
break;

View File

@ -1808,7 +1808,7 @@ void ModuleBitcodeWriter::writeDIFile(const DIFile *N,
}
auto Source = N->getRawSource();
if (Source)
Record.push_back(VE.getMetadataOrNullID(*Source));
Record.push_back(VE.getMetadataOrNullID(Source));
Stream.EmitRecord(bitc::METADATA_FILE, Record, Abbrev);
Record.clear();

View File

@ -801,8 +801,8 @@ DISubroutineType *DISubroutineType::getImpl(LLVMContext &Context, DIFlags Flags,
}
DIFile::DIFile(LLVMContext &C, StorageType Storage,
std::optional<ChecksumInfo<MDString *>> CS,
std::optional<MDString *> Src, ArrayRef<Metadata *> Ops)
std::optional<ChecksumInfo<MDString *>> CS, MDString *Src,
ArrayRef<Metadata *> Ops)
: DIScope(C, DIFileKind, Storage, dwarf::DW_TAG_file_type, Ops),
Checksum(CS), Source(Src) {}
@ -834,15 +834,15 @@ DIFile::getChecksumKind(StringRef CSKindStr) {
DIFile *DIFile::getImpl(LLVMContext &Context, MDString *Filename,
MDString *Directory,
std::optional<DIFile::ChecksumInfo<MDString *>> CS,
std::optional<MDString *> Source, StorageType Storage,
MDString *Source, StorageType Storage,
bool ShouldCreate) {
assert(isCanonical(Filename) && "Expected canonical MDString");
assert(isCanonical(Directory) && "Expected canonical MDString");
assert((!CS || isCanonical(CS->Value)) && "Expected canonical MDString");
assert((!Source || isCanonical(*Source)) && "Expected canonical MDString");
// We do *NOT* expect Source to be a canonical MDString because nullptr
// means none, so we need something to represent the empty file.
DEFINE_GETIMPL_LOOKUP(DIFile, (Filename, Directory, CS, Source));
Metadata *Ops[] = {Filename, Directory, CS ? CS->Value : nullptr,
Source.value_or(nullptr)};
Metadata *Ops[] = {Filename, Directory, CS ? CS->Value : nullptr, Source};
DEFINE_GETIMPL_STORE(DIFile, (CS, Source), Ops);
}
DICompileUnit::DICompileUnit(LLVMContext &C, StorageType Storage,

View File

@ -668,11 +668,11 @@ template <> struct MDNodeKeyImpl<DIFile> {
MDString *Filename;
MDString *Directory;
std::optional<DIFile::ChecksumInfo<MDString *>> Checksum;
std::optional<MDString *> Source;
MDString *Source;
MDNodeKeyImpl(MDString *Filename, MDString *Directory,
std::optional<DIFile::ChecksumInfo<MDString *>> Checksum,
std::optional<MDString *> Source)
MDString *Source)
: Filename(Filename), Directory(Directory), Checksum(Checksum),
Source(Source) {}
MDNodeKeyImpl(const DIFile *N)
@ -687,8 +687,7 @@ template <> struct MDNodeKeyImpl<DIFile> {
unsigned getHashValue() const {
return hash_combine(Filename, Directory, Checksum ? Checksum->Kind : 0,
Checksum ? Checksum->Value : nullptr,
Source.value_or(nullptr));
Checksum ? Checksum->Value : nullptr, Source);
}
};

View File

@ -190,6 +190,24 @@ TEST(MetadataTest, DeleteInstUsedByDbgValue) {
EXPECT_TRUE(isa<UndefValue>(DVIs[0]->getValue(0)));
}
TEST(DIBuiler, CreateFile) {
LLVMContext Ctx;
std::unique_ptr<Module> M(new Module("MyModule", Ctx));
DIBuilder DIB(*M);
DIFile *F = DIB.createFile("main.c", "/");
EXPECT_EQ(std::nullopt, F->getSource());
std::optional<DIFile::ChecksumInfo<StringRef>> Checksum = std::nullopt;
std::optional<StringRef> Source = std::nullopt;
F = DIB.createFile("main.c", "/", Checksum, Source);
EXPECT_EQ(Source, F->getSource());
Source = "";
F = DIB.createFile("main.c", "/", Checksum, Source);
EXPECT_EQ(Source, F->getSource());
}
TEST(DIBuilder, CreateFortranArrayTypeWithAttributes) {
LLVMContext Ctx;
std::unique_ptr<Module> M(new Module("MyModule", Ctx));

View File

@ -2221,6 +2221,20 @@ TEST_F(DIFileTest, get) {
EXPECT_EQ(N, MDNode::replaceWithUniqued(std::move(Temp)));
}
TEST_F(DIFileTest, EmptySource) {
DIFile *N = DIFile::get(Context, "file", "dir");
EXPECT_EQ(std::nullopt, N->getSource());
std::optional<DIFile::ChecksumInfo<StringRef>> Checksum = std::nullopt;
std::optional<StringRef> Source = std::nullopt;
N = DIFile::get(Context, "file", "dir", Checksum, Source);
EXPECT_EQ(Source, N->getSource());
Source = "";
N = DIFile::get(Context, "file", "dir", Checksum, Source);
EXPECT_EQ(Source, N->getSource());
}
TEST_F(DIFileTest, ScopeGetFile) {
// Ensure that DIScope::getFile() returns itself.
DIScope *N = DIFile::get(Context, "file", "dir");