Author: Jan Svoboda Date: 2021-03-05T11:11:05+01:00 New Revision: 4295ae96cdf275cdda8bded9271960a4cac11fb2
URL: https://github.com/llvm/llvm-project/commit/4295ae96cdf275cdda8bded9271960a4cac11fb2 DIFF: https://github.com/llvm/llvm-project/commit/4295ae96cdf275cdda8bded9271960a4cac11fb2.diff LOG: [clang][modules] Use extensible RTTI for ModuleFileExtension Clang exposes an interface for extending the PCM/PCH file format: `ModuleFileExtension`. Clang itself has only a single implementation of the interface: `TestModuleFileExtension` that can be instantiated via the `-ftest-module-file_extension=` command line argument (and is stored in `FrontendOptions::ModuleFileExtensions`). Clients of the Clang library can extend the PCM/PCH file format by pushing an instance of their extension class to the `FrontendOptions::ModuleFileExtensions` vector. When generating the `-ftest-module-file_extension=` command line argument from `FrontendOptions`, a downcast is used to distinguish between the Clang's testing extension and other (client) extensions. This functionality is enabled by LLVM-style RTTI. However, this style of RTTI is hard to extend, as it requires patching Clang (adding new case to the `ModuleFileExtensionKind` enum). This patch switches to the LLVM RTTI for open class hierarchies, which allows libClang users (e.g. Swift) to create implementations of `ModuleFileExtension` without patching Clang. (Documentation of the feature: https://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html#rtti-for-open-class-hierarchies) Reviewed By: artemcm Differential Revision: https://reviews.llvm.org/D97702 Added: Modified: clang/include/clang/Serialization/ModuleFileExtension.h clang/lib/Frontend/TestModuleFileExtension.cpp clang/lib/Frontend/TestModuleFileExtension.h clang/lib/Serialization/ModuleFileExtension.cpp clang/unittests/Frontend/CompilerInvocationTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Serialization/ModuleFileExtension.h b/clang/include/clang/Serialization/ModuleFileExtension.h index e9ac4c22520e..34ea870724a4 100644 --- a/clang/include/clang/Serialization/ModuleFileExtension.h +++ b/clang/include/clang/Serialization/ModuleFileExtension.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_SERIALIZATION_MODULEFILEEXTENSION_H #include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/Support/ExtensibleRTTI.h" #include <memory> #include <string> @@ -59,21 +60,16 @@ class ModuleFileExtensionWriter; /// compiled module files (.pcm) and precompiled headers (.pch) via a /// custom writer that can then be accessed via a custom reader when /// the module file or precompiled header is loaded. -class ModuleFileExtension { -protected: - /// Discriminator for LLVM-style RTTI. - enum ModuleFileExtensionKind { - MFEK_Test, - }; - - const ModuleFileExtensionKind Kind; +/// +/// Subclasses must use LLVM RTTI for open class hierarchies. +class ModuleFileExtension + : public llvm::RTTIExtends<ModuleFileExtension, llvm::RTTIRoot> { public: - ModuleFileExtension(ModuleFileExtensionKind Kind) : Kind(Kind) {} + /// Discriminator for LLVM RTTI. + static char ID; virtual ~ModuleFileExtension(); - ModuleFileExtensionKind getKind() const { return Kind; } - /// Retrieves the metadata for this module file extension. virtual ModuleFileExtensionMetadata getExtensionMetadata() const = 0; diff --git a/clang/lib/Frontend/TestModuleFileExtension.cpp b/clang/lib/Frontend/TestModuleFileExtension.cpp index ec147e16c569..7d4026a7efc6 100644 --- a/clang/lib/Frontend/TestModuleFileExtension.cpp +++ b/clang/lib/Frontend/TestModuleFileExtension.cpp @@ -15,6 +15,8 @@ using namespace clang; using namespace clang::serialization; +char TestModuleFileExtension::ID = 0; + TestModuleFileExtension::Writer::~Writer() { } void TestModuleFileExtension::Writer::writeExtensionContents( diff --git a/clang/lib/Frontend/TestModuleFileExtension.h b/clang/lib/Frontend/TestModuleFileExtension.h index ee7312502e24..c8ca4cd4f210 100644 --- a/clang/lib/Frontend/TestModuleFileExtension.h +++ b/clang/lib/Frontend/TestModuleFileExtension.h @@ -17,7 +17,8 @@ namespace clang { /// A module file extension used for testing purposes. -class TestModuleFileExtension : public ModuleFileExtension { +class TestModuleFileExtension + : public llvm::RTTIExtends<TestModuleFileExtension, ModuleFileExtension> { std::string BlockName; unsigned MajorVersion; unsigned MinorVersion; @@ -43,15 +44,13 @@ class TestModuleFileExtension : public ModuleFileExtension { }; public: - TestModuleFileExtension(StringRef BlockName, - unsigned MajorVersion, - unsigned MinorVersion, - bool Hashed, + static char ID; + + TestModuleFileExtension(StringRef BlockName, unsigned MajorVersion, + unsigned MinorVersion, bool Hashed, StringRef UserInfo) - : ModuleFileExtension(ModuleFileExtensionKind::MFEK_Test), - BlockName(BlockName), - MajorVersion(MajorVersion), MinorVersion(MinorVersion), - Hashed(Hashed), UserInfo(UserInfo) { } + : BlockName(BlockName), MajorVersion(MajorVersion), + MinorVersion(MinorVersion), Hashed(Hashed), UserInfo(UserInfo) {} ~TestModuleFileExtension() override; ModuleFileExtensionMetadata getExtensionMetadata() const override; @@ -66,10 +65,6 @@ class TestModuleFileExtension : public ModuleFileExtension { ASTReader &Reader, serialization::ModuleFile &Mod, const llvm::BitstreamCursor &Stream) override; - static bool classof(const ModuleFileExtension *E) { - return E->getKind() == MFEK_Test; - } - std::string str() const; }; diff --git a/clang/lib/Serialization/ModuleFileExtension.cpp b/clang/lib/Serialization/ModuleFileExtension.cpp index e1ae8a494ab1..6b7fd1d54340 100644 --- a/clang/lib/Serialization/ModuleFileExtension.cpp +++ b/clang/lib/Serialization/ModuleFileExtension.cpp @@ -9,6 +9,8 @@ #include "llvm/ADT/Hashing.h" using namespace clang; +char ModuleFileExtension::ID = 0; + ModuleFileExtension::~ModuleFileExtension() { } llvm::hash_code ModuleFileExtension::hashExtension(llvm::hash_code Code) const { diff --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp b/clang/unittests/Frontend/CompilerInvocationTest.cpp index b7f9cedc9cb3..89e024dcb78b 100644 --- a/clang/unittests/Frontend/CompilerInvocationTest.cpp +++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp @@ -11,6 +11,7 @@ #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/TextDiagnosticBuffer.h" #include "clang/Lex/PreprocessorOptions.h" +#include "clang/Serialization/ModuleFileExtension.h" #include "llvm/Support/Host.h" #include "gmock/gmock.h" @@ -743,6 +744,58 @@ TEST_F(CommandLineTest, DigraphsEnabled) { ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fdigraphs"))); } +struct DummyModuleFileExtension + : public llvm::RTTIExtends<DummyModuleFileExtension, ModuleFileExtension> { + static char ID; + + ModuleFileExtensionMetadata getExtensionMetadata() const override { + return {}; + }; + + llvm::hash_code hashExtension(llvm::hash_code Code) const override { + return {}; + } + + std::unique_ptr<ModuleFileExtensionWriter> + createExtensionWriter(ASTWriter &Writer) override { + return {}; + } + + std::unique_ptr<ModuleFileExtensionReader> + createExtensionReader(const ModuleFileExtensionMetadata &Metadata, + ASTReader &Reader, serialization::ModuleFile &Mod, + const llvm::BitstreamCursor &Stream) override { + return {}; + } +}; + +char DummyModuleFileExtension::ID = 0; + +TEST_F(CommandLineTest, TestModuleFileExtension) { + const char *Args[] = {"-ftest-module-file-extension=first:2:1:0:first", + "-ftest-module-file-extension=second:3:2:1:second"}; + + ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags)); + ASSERT_THAT(Invocation.getFrontendOpts().ModuleFileExtensions.size(), 2); + + // Exercise the check that only serializes instances of + // TestModuleFileExtension by providing an instance of another + // ModuleFileExtension subclass. + Invocation.getFrontendOpts().ModuleFileExtensions.push_back( + std::make_shared<DummyModuleFileExtension>()); + + Invocation.generateCC1CommandLine(GeneratedArgs, *this); + + ASSERT_THAT(GeneratedArgs, + ContainsN(HasSubstr("-ftest-module-file-extension="), 2)); + ASSERT_THAT( + GeneratedArgs, + Contains(StrEq("-ftest-module-file-extension=first:2:1:0:first"))); + ASSERT_THAT( + GeneratedArgs, + Contains(StrEq("-ftest-module-file-extension=second:3:2:1:second"))); +} + TEST_F(CommandLineTest, RoundTrip) { // Testing one marshalled and one manually generated option from each // CompilerInvocation member. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
