[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

diff --git a/clang/include/clang/Serialization/ModuleFileExtension.h b/clang/include/clang/Serialization/ModuleFileExtension.h
index e9ac4c2..34ea870 100644 (file)
--- 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 ec147e1..7d4026a 100644 (file)
--- 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 ee73125..c8ca4cd 100644 (file)
--- 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 @@ public:
                         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 e1ae8a4..6b7fd1d 100644 (file)
--- 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 b7f9ced..89e024d 100644 (file)
--- 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.