[clang][modules] Use extensible RTTI for ModuleFileExtension

[jansvoboda11]

Summary

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)

[Bigcheese]

Looks fine to me, but it would be good for a Swift person to look at this patch.

[artemcm]

This looks good to me, thanks!

clang/include/clang/Serialization/ModuleFileExtension.h

 //===-- ModuleFileExtension.h - Module File Extensions ----------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_SERIALIZATION_MODULEFILEEXTENSION_H
 #define LLVM_CLANG_SERIALIZATION_MODULEFILEEXTENSION_H
 
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/ExtensibleRTTI.h"
 #include <memory>
 #include <string>
 
 namespace llvm {
 class BitstreamCursor;
 class BitstreamWriter;
 class hash_code;
 class raw_ostream;

 
 /// An abstract superclass that describes a custom extension to the
 /// module/precompiled header file format.
 ///
 /// A module file extension can introduce additional information into
 /// 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;
 
   /// Hash information about the presence of this extension into the
   /// module hash code.
   ///
   /// The module hash code is used to distinguish different variants
   /// of a module that are incompatible. If the presence, absence, or

clang/lib/Frontend/TestModuleFileExtension.h

 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Bitstream/BitstreamReader.h"
 #include <string>
 
 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;
   bool Hashed;
   std::string UserInfo;
 
   class Writer : public ModuleFileExtensionWriter {
   public:
class TestModuleFileExtension
 
   public:
     ~Reader() override;
 
     Reader(ModuleFileExtension *Ext, const llvm::BitstreamCursor &InStream);
   };
 
 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;
 
   llvm::hash_code hashExtension(llvm::hash_code Code) const override;
 
   std::unique_ptr<ModuleFileExtensionWriter>
   createExtensionWriter(ASTWriter &Writer) override;
 
   std::unique_ptr<ModuleFileExtensionReader>
   createExtensionReader(const ModuleFileExtensionMetadata &Metadata,
                         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;
 };
 
 } // end namespace clang
 
 #endif // LLVM_CLANG_FRONTEND_TESTMODULEFILEEXTENSION_H

clang/lib/Frontend/TestModuleFileExtension.cpp

 #include "clang/Serialization/ASTReader.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/Bitstream/BitstreamWriter.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cstdio>
 using namespace clang;
 using namespace clang::serialization;
 
+char TestModuleFileExtension::ID = 0;
+
 TestModuleFileExtension::Writer::~Writer() { }
 
 void TestModuleFileExtension::Writer::writeExtensionContents(
        Sema &SemaRef,
        llvm::BitstreamWriter &Stream) {
   using namespace llvm;
 
   // Write an abbreviation for this record.

clang/lib/Serialization/ModuleFileExtension.cpp

 //===-- ModuleFileExtension.cpp - Module File Extensions ------------------===//
 //
 // 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/Serialization/ModuleFileExtension.h"
 #include "llvm/ADT/Hashing.h"
 using namespace clang;
 
+char ModuleFileExtension::ID = 0;
+
 ModuleFileExtension::~ModuleFileExtension() { }
 
 llvm::hash_code ModuleFileExtension::hashExtension(llvm::hash_code Code) const {
   return Code;
 }
 
 ModuleFileExtensionWriter::~ModuleFileExtensionWriter() { }
 
 ModuleFileExtensionReader::~ModuleFileExtensionReader() { }

clang/unittests/Frontend/CompilerInvocationTest.cpp

 //===- unittests/Frontend/CompilerInvocationTest.cpp - CI tests //---------===//
 //
 // 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/CompilerInvocation.h"
 #include "clang/Basic/TargetOptions.h"
 #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"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 using namespace clang;
 
TEST_F(CommandLineTest, DigraphsEnabled) {
 
   ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
   ASSERT_TRUE(Invocation.getLangOpts()->Digraphs);
 
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
   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.
   const char *Args[] = {
       "-round-trip-args",
       // LanguageOptions
       "-std=c17",
       "-fmax-tokens=10",