[llvm] c9cb4fc - [DebugInfo] Store optional DIFile::Source as pointer
Jonas Hahnfeld via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 8 00:59:10 PST 2022
Author: Jonas Hahnfeld
Date: 2022-12-08T09:58:33+01:00
New Revision: c9cb4fc761cd78d3a04feb211d363e91933a8e08
URL: https://github.com/llvm/llvm-project/commit/c9cb4fc761cd78d3a04feb211d363e91933a8e08
DIFF: https://github.com/llvm/llvm-project/commit/c9cb4fc761cd78d3a04feb211d363e91933a8e08.diff
LOG: [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
Added:
Modified:
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/MetadataLoader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/lib/IR/LLVMContextImpl.h
llvm/unittests/IR/DebugInfoTest.cpp
llvm/unittests/IR/MetadataTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index f9e11345a739c..d2ecae606b469 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -598,11 +598,12 @@ class DIFile : public DIScope {
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 @@ class DIFile : public DIScope {
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 @@ class DIFile : public DIScope {
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 @@ class DIFile : public DIScope {
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 @@ class DIFile : public DIScope {
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);
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index 76cb5ee6944f8..f02e8cb52d1fe 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -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;
}
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 0f3ad255f6b72..96f436a9b137d 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -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;
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 9c45ea00e8a09..28b1bac8960fd 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -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();
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index 424c44f0055e9..2d220cc133093 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -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,
diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h
index 99270daf1974c..eadb3b7fba775 100644
--- a/llvm/lib/IR/LLVMContextImpl.h
+++ b/llvm/lib/IR/LLVMContextImpl.h
@@ -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);
}
};
diff --git a/llvm/unittests/IR/DebugInfoTest.cpp b/llvm/unittests/IR/DebugInfoTest.cpp
index 439c0d00ee4eb..1930ecdd8cc9f 100644
--- a/llvm/unittests/IR/DebugInfoTest.cpp
+++ b/llvm/unittests/IR/DebugInfoTest.cpp
@@ -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));
diff --git a/llvm/unittests/IR/MetadataTest.cpp b/llvm/unittests/IR/MetadataTest.cpp
index 3ffa45663d63b..982e48ba701bf 100644
--- a/llvm/unittests/IR/MetadataTest.cpp
+++ b/llvm/unittests/IR/MetadataTest.cpp
@@ -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");
More information about the llvm-commits
mailing list