diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 1fadd8039462..321c11e55c14 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -800,11 +800,12 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) { BitsUnpacker EnumDeclBits(Record.readInt()); ED->setNumPositiveBits(EnumDeclBits.getNextBits(/*Width=*/8)); ED->setNumNegativeBits(EnumDeclBits.getNextBits(/*Width=*/8)); + bool ShouldSkipCheckingODR = EnumDeclBits.getNextBit(); ED->setScoped(EnumDeclBits.getNextBit()); ED->setScopedUsingClassTag(EnumDeclBits.getNextBit()); ED->setFixed(EnumDeclBits.getNextBit()); - if (!shouldSkipCheckingODR(ED)) { + if (!ShouldSkipCheckingODR) { ED->setHasODRHash(true); ED->ODRHash = Record.readInt(); } @@ -1073,6 +1074,7 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) { FD->setCachedLinkage((Linkage)FunctionDeclBits.getNextBits(/*Width=*/3)); FD->setStorageClass((StorageClass)FunctionDeclBits.getNextBits(/*Width=*/3)); + bool ShouldSkipCheckingODR = FunctionDeclBits.getNextBit(); FD->setInlineSpecified(FunctionDeclBits.getNextBit()); FD->setImplicitlyInline(FunctionDeclBits.getNextBit()); FD->setHasSkippedBody(FunctionDeclBits.getNextBit()); @@ -1102,7 +1104,7 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) { if (FD->isExplicitlyDefaulted()) FD->setDefaultLoc(readSourceLocation()); - if (!shouldSkipCheckingODR(FD)) { + if (!ShouldSkipCheckingODR) { FD->ODRHash = Record.readInt(); FD->setHasODRHash(true); } @@ -1973,6 +1975,8 @@ void ASTDeclReader::ReadCXXDefinitionData( BitsUnpacker CXXRecordDeclBits = Record.readInt(); + bool ShouldSkipCheckingODR = CXXRecordDeclBits.getNextBit(); + #define FIELD(Name, Width, Merge) \ if (!CXXRecordDeclBits.canGetNextNBits(Width)) \ CXXRecordDeclBits.updateValue(Record.readInt()); \ @@ -1982,7 +1986,7 @@ void ASTDeclReader::ReadCXXDefinitionData( #undef FIELD // We only perform ODR checks for decls not in GMF. - if (!shouldSkipCheckingODR(D)) { + if (!ShouldSkipCheckingODR) { // Note: the caller has deserialized the IsLambda bit already. Data.ODRHash = Record.readInt(); Data.HasODRHash = true; diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 3b79a9238d1a..73018c1170d8 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -6010,6 +6010,9 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) { BitsPacker DefinitionBits; + bool ShouldSkipCheckingODR = shouldSkipCheckingODR(D); + DefinitionBits.addBit(ShouldSkipCheckingODR); + #define FIELD(Name, Width, Merge) \ if (!DefinitionBits.canWriteNextNBits(Width)) { \ Record->push_back(DefinitionBits); \ @@ -6023,11 +6026,10 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) { Record->push_back(DefinitionBits); // We only perform ODR checks for decls not in GMF. - if (!shouldSkipCheckingODR(D)) { + if (!ShouldSkipCheckingODR) // getODRHash will compute the ODRHash if it has not been previously // computed. Record->push_back(D->getODRHash()); - } bool ModulesDebugInfo = Writer->Context->getLangOpts().ModulesDebugInfo && !D->isDependentType(); diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index f224075643e9..e73800100e3c 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -488,13 +488,15 @@ void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) { BitsPacker EnumDeclBits; EnumDeclBits.addBits(D->getNumPositiveBits(), /*BitWidth=*/8); EnumDeclBits.addBits(D->getNumNegativeBits(), /*BitWidth=*/8); + bool ShouldSkipCheckingODR = shouldSkipCheckingODR(D); + EnumDeclBits.addBit(ShouldSkipCheckingODR); EnumDeclBits.addBit(D->isScoped()); EnumDeclBits.addBit(D->isScopedUsingClassTag()); EnumDeclBits.addBit(D->isFixed()); Record.push_back(EnumDeclBits); // We only perform ODR checks for decls not in GMF. - if (!shouldSkipCheckingODR(D)) + if (!ShouldSkipCheckingODR) Record.push_back(D->getODRHash()); if (MemberSpecializationInfo *MemberInfo = D->getMemberSpecializationInfo()) { @@ -678,6 +680,8 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) { // FIXME: stable encoding FunctionDeclBits.addBits(llvm::to_underlying(D->getLinkageInternal()), 3); FunctionDeclBits.addBits((uint32_t)D->getStorageClass(), /*BitWidth=*/3); + bool ShouldSkipCheckingODR = shouldSkipCheckingODR(D); + FunctionDeclBits.addBit(ShouldSkipCheckingODR); FunctionDeclBits.addBit(D->isInlineSpecified()); FunctionDeclBits.addBit(D->isInlined()); FunctionDeclBits.addBit(D->hasSkippedBody()); @@ -704,7 +708,7 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) { Record.AddSourceLocation(D->getDefaultLoc()); // We only perform ODR checks for decls not in GMF. - if (!shouldSkipCheckingODR(D)) + if (!ShouldSkipCheckingODR) Record.push_back(D->getODRHash()); if (D->isDefaulted()) { @@ -2137,12 +2141,13 @@ getFunctionDeclAbbrev(serialization::DeclCode Code) { Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 11)); // IDNS Abv->Add(BitCodeAbbrevOp( BitCodeAbbrevOp::Fixed, - 27)); // Packed Function Bits: StorageClass, Inline, InlineSpecified, + 28)); // Packed Function Bits: StorageClass, Inline, InlineSpecified, // VirtualAsWritten, Pure, HasInheritedProto, HasWrittenProto, // Deleted, Trivial, TrivialForCall, Defaulted, ExplicitlyDefaulted, // IsIneligibleOrNotSelected, ImplicitReturnZero, Constexpr, // UsesSEHTry, SkippedBody, MultiVersion, LateParsed, - // FriendConstraintRefersToEnclosingTemplate, Linkage + // FriendConstraintRefersToEnclosingTemplate, Linkage, + // ShouldSkipCheckingODR Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // LocEnd Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32)); // ODRHash // This Array slurps the rest of the record. Fortunately we want to encode @@ -2269,7 +2274,7 @@ void ASTWriter::WriteDeclAbbrevs() { Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // AddTypeRef Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // IntegerType Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // getPromotionType - Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 19)); // Enum Decl Bits + Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 20)); // Enum Decl Bits Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));// ODRHash Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // InstantiatedMembEnum // DC diff --git a/clang/unittests/Serialization/CMakeLists.txt b/clang/unittests/Serialization/CMakeLists.txt index 10d7de970c64..e7eebd0cb982 100644 --- a/clang/unittests/Serialization/CMakeLists.txt +++ b/clang/unittests/Serialization/CMakeLists.txt @@ -10,6 +10,7 @@ add_clang_unittest(SerializationTests InMemoryModuleCacheTest.cpp ModuleCacheTest.cpp NoCommentsTest.cpp + PreambleInNamedModulesTest.cpp SourceLocationEncodingTest.cpp VarDeclConstantInitTest.cpp ) diff --git a/clang/unittests/Serialization/PreambleInNamedModulesTest.cpp b/clang/unittests/Serialization/PreambleInNamedModulesTest.cpp new file mode 100644 index 000000000000..d26e1cb63365 --- /dev/null +++ b/clang/unittests/Serialization/PreambleInNamedModulesTest.cpp @@ -0,0 +1,132 @@ +//===- unittests/Serialization/PreambleInNamedModulesTest.cpp -------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/CompilerInvocation.h" +#include "clang/Frontend/FrontendActions.h" +#include "clang/Frontend/PrecompiledPreamble.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/raw_ostream.h" + +#include "gtest/gtest.h" + +using namespace llvm; +using namespace clang; + +namespace { + +class PreambleInNamedModulesTest : public ::testing::Test { + void SetUp() override { + ASSERT_FALSE(sys::fs::createUniqueDirectory("modules-test", TestDir)); + } + + void TearDown() override { sys::fs::remove_directories(TestDir); } + +public: + using PathType = SmallString<256>; + + PathType TestDir; + + void addFile(StringRef Path, StringRef Contents, PathType &AbsPath) { + ASSERT_FALSE(sys::path::is_absolute(Path)); + + AbsPath = TestDir; + sys::path::append(AbsPath, Path); + + ASSERT_FALSE( + sys::fs::create_directories(llvm::sys::path::parent_path(AbsPath))); + + std::error_code EC; + llvm::raw_fd_ostream OS(AbsPath, EC); + ASSERT_FALSE(EC); + OS << Contents; + } + + void addFile(StringRef Path, StringRef Contents) { + PathType UnusedAbsPath; + addFile(Path, Contents, UnusedAbsPath); + } +}; + +// Testing that the use of Preamble in named modules can work basically. +// See https://github.com/llvm/llvm-project/issues/80570 +TEST_F(PreambleInNamedModulesTest, BasicTest) { + addFile("foo.h", R"cpp( +enum class E { + A, + B, + C, + D +}; + )cpp"); + + PathType MainFilePath; + addFile("A.cppm", R"cpp( +module; +#include "foo.h" +export module A; +export using ::E; + )cpp", + MainFilePath); + + IntrusiveRefCntPtr Diags = + CompilerInstance::createDiagnostics(new DiagnosticOptions()); + IntrusiveRefCntPtr VFS = + llvm::vfs::createPhysicalFileSystem(); + + CreateInvocationOptions CIOpts; + CIOpts.Diags = Diags; + CIOpts.VFS = VFS; + + const char *Args[] = {"clang++", "-std=c++20", "-working-directory", + TestDir.c_str(), MainFilePath.c_str()}; + std::shared_ptr Invocation = + createInvocation(Args, CIOpts); + ASSERT_TRUE(Invocation); + + llvm::ErrorOr> ContentsBuffer = + llvm::MemoryBuffer::getFile(MainFilePath, /*IsText=*/true); + EXPECT_TRUE(ContentsBuffer); + std::unique_ptr Buffer = std::move(*ContentsBuffer); + + PreambleBounds Bounds = + ComputePreambleBounds(Invocation->getLangOpts(), *Buffer, 0); + + PreambleCallbacks Callbacks; + llvm::ErrorOr BuiltPreamble = PrecompiledPreamble::Build( + *Invocation, Buffer.get(), Bounds, *Diags, VFS, + std::make_shared(), + /*StoreInMemory=*/false, /*StoragePath=*/TestDir, Callbacks); + + ASSERT_FALSE(Diags->hasErrorOccurred()); + + EXPECT_TRUE(BuiltPreamble); + EXPECT_TRUE(BuiltPreamble->CanReuse(*Invocation, *Buffer, Bounds, *VFS)); + BuiltPreamble->OverridePreamble(*Invocation, VFS, Buffer.get()); + + auto Clang = std::make_unique( + std::make_shared()); + Clang->setInvocation(std::move(Invocation)); + Clang->setDiagnostics(Diags.get()); + + if (auto VFSWithRemapping = createVFSFromCompilerInvocation( + Clang->getInvocation(), Clang->getDiagnostics(), VFS)) + VFS = VFSWithRemapping; + + Clang->createFileManager(VFS); + EXPECT_TRUE(Clang->createTarget()); + + Buffer.release(); + + SyntaxOnlyAction Action; + EXPECT_TRUE(Clang->ExecuteAction(Action)); + EXPECT_FALSE(Clang->getDiagnosticsPtr()->hasErrorOccurred()); +} + +} // namespace