[clang] 4295ae9 - [clang][modules] Use extensible RTTI for ModuleFileExtension
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 5 02:11:21 PST 2021
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.
More information about the cfe-commits
mailing list