This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a115578324f: [clang][modules] Avoid storing command-line
macro definitions into implicitly… (authored by jansvoboda11).
Changed prior to commit:
https://reviews.llvm.org/D158136?vs=550964&id=551197#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158136/new/
https://reviews.llvm.org/D158136
Files:
clang/include/clang/Serialization/ASTReader.h
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/GeneratePCH.cpp
clang/test/Modules/module_file_info.m
Index: clang/test/Modules/module_file_info.m
===================================================================
--- clang/test/Modules/module_file_info.m
+++ clang/test/Modules/module_file_info.m
@@ -1,14 +1,15 @@
// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
@import DependsOnModule;
-// RUN: rm -rf %t %t-obj
+// RUN: rm -rf %t %t-obj %t-hash
// RUN: %clang_cc1 -w -Wunused -fmodules -fmodule-format=raw -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t -F %S/Inputs -DBLARG -DWIBBLE=WOBBLE -fmodule-feature myfeature %s
-// RUN: %clang_cc1 -module-file-info %t/DependsOnModule.pcm | FileCheck %s
-// RUN: %clang_cc1 -module-file-info %t/DependsOnModule.pcm | FileCheck %s --check-prefix=RAW
+// RUN: %clang_cc1 -module-file-info %t/DependsOnModule.pcm | FileCheck %s --check-prefixes=RAW,CHECK,MACROS
// RUN: %clang_cc1 -w -Wunused -fmodules -fmodule-format=obj -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-obj -F %S/Inputs -DBLARG -DWIBBLE=WOBBLE -fmodule-feature myfeature %s
-// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
-// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s --check-prefix=OBJ
+// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s --check-prefixes=OBJ,CHECK,MACROS
+
+// RUN: %clang_cc1 -w -Wunused -fmodules -fmodule-format=obj -fimplicit-module-maps -fmodules-cache-path=%t-hash -F %S/Inputs -DBLARG -DWIBBLE=WOBBLE -fmodule-feature myfeature %s
+// RUN: %clang_cc1 -module-file-info %t-hash/*/DependsOnModule-*.pcm | FileCheck %s --check-prefixes=OBJ,CHECK,NO_MACROS
// RAW: Module format: raw
// OBJ: Module format: obj
@@ -16,7 +17,7 @@
// CHECK: Module name: DependsOnModule
// CHECK: Module map file: {{.*}}DependsOnModule.framework{{[/\\]}}module.map
-// CHECK: Imports module 'Module': {{.*}}Module.pcm
+// CHECK: Imports module 'Module': {{.*}}Module{{.*}}.pcm
// CHECK: Language options:
// CHECK: C99: Yes
@@ -42,9 +43,10 @@
// CHECK: Preprocessor options:
// CHECK: Uses compiler/target-specific predefines [-undef]: Yes
// CHECK: Uses detailed preprocessing record (for indexing): No
-// CHECK: Predefined macros:
-// CHECK: -DBLARG
-// CHECK: -DWIBBLE=WOBBLE
+// NO_MACROS-NOT: Predefined macros:
+// MACROS: Predefined macros:
+// MACROS-NEXT: -DBLARG
+// MACROS-NEXT: -DWIBBLE=WOBBLE
// CHECK: Input file: {{.*}}module.map
// CHECK-NEXT: Input file: {{.*}}module_private.map
// CHECK-NEXT: Input file: {{.*}}DependsOnModule.h
Index: clang/lib/Serialization/GeneratePCH.cpp
===================================================================
--- clang/lib/Serialization/GeneratePCH.cpp
+++ clang/lib/Serialization/GeneratePCH.cpp
@@ -25,11 +25,11 @@
StringRef OutputFile, StringRef isysroot, std::shared_ptr<PCHBuffer> Buffer,
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
bool AllowASTWithErrors, bool IncludeTimestamps,
- bool ShouldCacheASTInMemory)
+ bool BuildingImplicitModule, bool ShouldCacheASTInMemory)
: PP(PP), OutputFile(OutputFile), isysroot(isysroot.str()),
SemaPtr(nullptr), Buffer(std::move(Buffer)), Stream(this->Buffer->Data),
Writer(Stream, this->Buffer->Data, ModuleCache, Extensions,
- IncludeTimestamps),
+ IncludeTimestamps, BuildingImplicitModule),
AllowASTWithErrors(AllowASTWithErrors),
ShouldCacheASTInMemory(ShouldCacheASTInMemory) {
this->Buffer->IsComplete = false;
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1467,11 +1467,19 @@
Record.clear();
const PreprocessorOptions &PPOpts = PP.getPreprocessorOpts();
- // Macro definitions.
- Record.push_back(PPOpts.Macros.size());
- for (unsigned I = 0, N = PPOpts.Macros.size(); I != N; ++I) {
- AddString(PPOpts.Macros[I].first, Record);
- Record.push_back(PPOpts.Macros[I].second);
+ // If we're building an implicit module with a context hash, the importer is
+ // guaranteed to have the same macros defined on the command line. Skip
+ // writing them.
+ bool SkipMacros = BuildingImplicitModule && !HSOpts.DisableModuleHash;
+ bool WriteMacros = !SkipMacros;
+ Record.push_back(WriteMacros);
+ if (WriteMacros) {
+ // Macro definitions.
+ Record.push_back(PPOpts.Macros.size());
+ for (unsigned I = 0, N = PPOpts.Macros.size(); I != N; ++I) {
+ AddString(PPOpts.Macros[I].first, Record);
+ Record.push_back(PPOpts.Macros[I].second);
+ }
}
// Includes
@@ -4540,9 +4548,10 @@
SmallVectorImpl<char> &Buffer,
InMemoryModuleCache &ModuleCache,
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
- bool IncludeTimestamps)
+ bool IncludeTimestamps, bool BuildingImplicitModule)
: Stream(Stream), Buffer(Buffer), ModuleCache(ModuleCache),
- IncludeTimestamps(IncludeTimestamps) {
+ IncludeTimestamps(IncludeTimestamps),
+ BuildingImplicitModule(BuildingImplicitModule) {
for (const auto &Ext : Extensions) {
if (auto Writer = Ext->createExtensionWriter(*this))
ModuleFileExtensionWriters.push_back(std::move(Writer));
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -208,11 +208,12 @@
}
bool ChainedASTReaderListener::ReadPreprocessorOptions(
- const PreprocessorOptions &PPOpts, bool Complain,
+ const PreprocessorOptions &PPOpts, bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) {
- return First->ReadPreprocessorOptions(PPOpts, Complain,
+ return First->ReadPreprocessorOptions(PPOpts, ReadMacros, Complain,
SuggestedPredefines) ||
- Second->ReadPreprocessorOptions(PPOpts, Complain, SuggestedPredefines);
+ Second->ReadPreprocessorOptions(PPOpts, ReadMacros, Complain,
+ SuggestedPredefines);
}
void ChainedASTReaderListener::ReadCounter(const serialization::ModuleFile &M,
@@ -658,92 +659,95 @@
/// are no differences in the options between the two.
static bool checkPreprocessorOptions(
const PreprocessorOptions &PPOpts,
- const PreprocessorOptions &ExistingPPOpts, DiagnosticsEngine *Diags,
- FileManager &FileMgr, std::string &SuggestedPredefines,
- const LangOptions &LangOpts,
+ const PreprocessorOptions &ExistingPPOpts, bool ReadMacros,
+ DiagnosticsEngine *Diags, FileManager &FileMgr,
+ std::string &SuggestedPredefines, const LangOptions &LangOpts,
OptionValidation Validation = OptionValidateContradictions) {
- // Check macro definitions.
- MacroDefinitionsMap ASTFileMacros;
- collectMacroDefinitions(PPOpts, ASTFileMacros);
- MacroDefinitionsMap ExistingMacros;
- SmallVector<StringRef, 4> ExistingMacroNames;
- collectMacroDefinitions(ExistingPPOpts, ExistingMacros, &ExistingMacroNames);
-
- // Use a line marker to enter the <command line> file, as the defines and
- // undefines here will have come from the command line.
- SuggestedPredefines += "# 1 \"<command line>\" 1\n";
-
- for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) {
- // Dig out the macro definition in the existing preprocessor options.
- StringRef MacroName = ExistingMacroNames[I];
- std::pair<StringRef, bool> Existing = ExistingMacros[MacroName];
-
- // Check whether we know anything about this macro name or not.
- llvm::StringMap<std::pair<StringRef, bool /*IsUndef*/>>::iterator Known =
- ASTFileMacros.find(MacroName);
- if (Validation == OptionValidateNone || Known == ASTFileMacros.end()) {
- if (Validation == OptionValidateStrictMatches) {
- // If strict matches are requested, don't tolerate any extra defines on
- // the command line that are missing in the AST file.
+ if (ReadMacros) {
+ // Check macro definitions.
+ MacroDefinitionsMap ASTFileMacros;
+ collectMacroDefinitions(PPOpts, ASTFileMacros);
+ MacroDefinitionsMap ExistingMacros;
+ SmallVector<StringRef, 4> ExistingMacroNames;
+ collectMacroDefinitions(ExistingPPOpts, ExistingMacros,
+ &ExistingMacroNames);
+
+ // Use a line marker to enter the <command line> file, as the defines and
+ // undefines here will have come from the command line.
+ SuggestedPredefines += "# 1 \"<command line>\" 1\n";
+
+ for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) {
+ // Dig out the macro definition in the existing preprocessor options.
+ StringRef MacroName = ExistingMacroNames[I];
+ std::pair<StringRef, bool> Existing = ExistingMacros[MacroName];
+
+ // Check whether we know anything about this macro name or not.
+ llvm::StringMap<std::pair<StringRef, bool /*IsUndef*/>>::iterator Known =
+ ASTFileMacros.find(MacroName);
+ if (Validation == OptionValidateNone || Known == ASTFileMacros.end()) {
+ if (Validation == OptionValidateStrictMatches) {
+ // If strict matches are requested, don't tolerate any extra defines
+ // on the command line that are missing in the AST file.
+ if (Diags) {
+ Diags->Report(diag::err_pch_macro_def_undef) << MacroName << true;
+ }
+ return true;
+ }
+ // FIXME: Check whether this identifier was referenced anywhere in the
+ // AST file. If so, we should reject the AST file. Unfortunately, this
+ // information isn't in the control block. What shall we do about it?
+
+ if (Existing.second) {
+ SuggestedPredefines += "#undef ";
+ SuggestedPredefines += MacroName.str();
+ SuggestedPredefines += '\n';
+ } else {
+ SuggestedPredefines += "#define ";
+ SuggestedPredefines += MacroName.str();
+ SuggestedPredefines += ' ';
+ SuggestedPredefines += Existing.first.str();
+ SuggestedPredefines += '\n';
+ }
+ continue;
+ }
+
+ // If the macro was defined in one but undef'd in the other, we have a
+ // conflict.
+ if (Existing.second != Known->second.second) {
if (Diags) {
- Diags->Report(diag::err_pch_macro_def_undef) << MacroName << true;
+ Diags->Report(diag::err_pch_macro_def_undef)
+ << MacroName << Known->second.second;
}
return true;
}
- // FIXME: Check whether this identifier was referenced anywhere in the
- // AST file. If so, we should reject the AST file. Unfortunately, this
- // information isn't in the control block. What shall we do about it?
-
- if (Existing.second) {
- SuggestedPredefines += "#undef ";
- SuggestedPredefines += MacroName.str();
- SuggestedPredefines += '\n';
- } else {
- SuggestedPredefines += "#define ";
- SuggestedPredefines += MacroName.str();
- SuggestedPredefines += ' ';
- SuggestedPredefines += Existing.first.str();
- SuggestedPredefines += '\n';
+
+ // If the macro was #undef'd in both, or if the macro bodies are
+ // identical, it's fine.
+ if (Existing.second || Existing.first == Known->second.first) {
+ ASTFileMacros.erase(Known);
+ continue;
}
- continue;
- }
- // If the macro was defined in one but undef'd in the other, we have a
- // conflict.
- if (Existing.second != Known->second.second) {
+ // The macro bodies differ; complain.
if (Diags) {
- Diags->Report(diag::err_pch_macro_def_undef)
- << MacroName << Known->second.second;
+ Diags->Report(diag::err_pch_macro_def_conflict)
+ << MacroName << Known->second.first << Existing.first;
}
return true;
}
- // If the macro was #undef'd in both, or if the macro bodies are identical,
- // it's fine.
- if (Existing.second || Existing.first == Known->second.first) {
- ASTFileMacros.erase(Known);
- continue;
- }
-
- // The macro bodies differ; complain.
- if (Diags) {
- Diags->Report(diag::err_pch_macro_def_conflict)
- << MacroName << Known->second.first << Existing.first;
- }
- return true;
- }
-
- // Leave the <command line> file and return to <built-in>.
- SuggestedPredefines += "# 1 \"<built-in>\" 2\n";
+ // Leave the <command line> file and return to <built-in>.
+ SuggestedPredefines += "# 1 \"<built-in>\" 2\n";
- if (Validation == OptionValidateStrictMatches) {
- // If strict matches are requested, don't tolerate any extra defines in
- // the AST file that are missing on the command line.
- for (const auto &MacroName : ASTFileMacros.keys()) {
- if (Diags) {
- Diags->Report(diag::err_pch_macro_def_undef) << MacroName << false;
+ if (Validation == OptionValidateStrictMatches) {
+ // If strict matches are requested, don't tolerate any extra defines in
+ // the AST file that are missing on the command line.
+ for (const auto &MacroName : ASTFileMacros.keys()) {
+ if (Diags) {
+ Diags->Report(diag::err_pch_macro_def_undef) << MacroName << false;
+ }
+ return true;
}
- return true;
}
}
@@ -805,24 +809,22 @@
}
bool PCHValidator::ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
- bool Complain,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) {
const PreprocessorOptions &ExistingPPOpts = PP.getPreprocessorOpts();
- return checkPreprocessorOptions(PPOpts, ExistingPPOpts,
- Complain? &Reader.Diags : nullptr,
- PP.getFileManager(),
- SuggestedPredefines,
- PP.getLangOpts());
+ return checkPreprocessorOptions(
+ PPOpts, ExistingPPOpts, ReadMacros, Complain ? &Reader.Diags : nullptr,
+ PP.getFileManager(), SuggestedPredefines, PP.getLangOpts());
}
bool SimpleASTReaderListener::ReadPreprocessorOptions(
- const PreprocessorOptions &PPOpts,
- bool Complain,
- std::string &SuggestedPredefines) {
- return checkPreprocessorOptions(PPOpts, PP.getPreprocessorOpts(), nullptr,
- PP.getFileManager(), SuggestedPredefines,
- PP.getLangOpts(), OptionValidateNone);
+ const PreprocessorOptions &PPOpts, bool ReadMacros, bool Complain,
+ std::string &SuggestedPredefines) {
+ return checkPreprocessorOptions(PPOpts, PP.getPreprocessorOpts(), ReadMacros,
+ nullptr, PP.getFileManager(),
+ SuggestedPredefines, PP.getLangOpts(),
+ OptionValidateNone);
}
/// Check the header search options deserialized from the control block
@@ -5274,10 +5276,10 @@
}
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
- bool Complain,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override {
return checkPreprocessorOptions(
- PPOpts, ExistingPPOpts, /*Diags=*/nullptr, FileMgr,
+ PPOpts, ExistingPPOpts, ReadMacros, /*Diags=*/nullptr, FileMgr,
SuggestedPredefines, ExistingLangOpts,
StrictOptionMatches ? OptionValidateStrictMatches
: OptionValidateContradictions);
@@ -6048,10 +6050,13 @@
unsigned Idx = 0;
// Macro definitions/undefs
- for (unsigned N = Record[Idx++]; N; --N) {
- std::string Macro = ReadString(Record, Idx);
- bool IsUndef = Record[Idx++];
- PPOpts.Macros.push_back(std::make_pair(Macro, IsUndef));
+ bool ReadMacros = Record[Idx++];
+ if (ReadMacros) {
+ for (unsigned N = Record[Idx++]; N; --N) {
+ std::string Macro = ReadString(Record, Idx);
+ bool IsUndef = Record[Idx++];
+ PPOpts.Macros.push_back(std::make_pair(Macro, IsUndef));
+ }
}
// Includes
@@ -6070,7 +6075,7 @@
PPOpts.ObjCXXARCStandardLibrary =
static_cast<ObjCXXARCStandardLibraryKind>(Record[Idx++]);
SuggestedPredefines.clear();
- return Listener.ReadPreprocessorOptions(PPOpts, Complain,
+ return Listener.ReadPreprocessorOptions(PPOpts, ReadMacros, Complain,
SuggestedPredefines);
}
Index: clang/lib/Frontend/FrontendActions.cpp
===================================================================
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -141,7 +141,8 @@
CI.getPreprocessor(), CI.getModuleCache(), OutputFile, Sysroot, Buffer,
FrontendOpts.ModuleFileExtensions,
CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
- FrontendOpts.IncludeTimestamps, +CI.getLangOpts().CacheGeneratedPCH));
+ FrontendOpts.IncludeTimestamps, FrontendOpts.BuildingImplicitModule,
+ +CI.getLangOpts().CacheGeneratedPCH));
Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
CI, std::string(InFile), OutputFile, std::move(OS), Buffer));
@@ -204,6 +205,7 @@
/*IncludeTimestamps=*/
+CI.getFrontendOpts().BuildingImplicitModule &&
+CI.getFrontendOpts().IncludeTimestamps,
+ /*BuildingImplicitModule=*/+CI.getFrontendOpts().BuildingImplicitModule,
/*ShouldCacheASTInMemory=*/
+CI.getFrontendOpts().BuildingImplicitModule));
Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
@@ -660,7 +662,7 @@
}
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
- bool Complain,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override {
Out.indent(2) << "Preprocessor options:\n";
DUMP_BOOLEAN(PPOpts.UsePredefines,
@@ -668,7 +670,7 @@
DUMP_BOOLEAN(PPOpts.DetailedRecord,
"Uses detailed preprocessing record (for indexing)");
- if (!PPOpts.Macros.empty()) {
+ if (ReadMacros) {
Out.indent(4) << "Predefined macros:\n";
}
Index: clang/lib/Frontend/ASTUnit.cpp
===================================================================
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -581,7 +581,8 @@
return false;
}
- bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain,
+ bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override {
this->PPOpts = PPOpts;
return false;
Index: clang/include/clang/Serialization/ASTWriter.h
===================================================================
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -143,6 +143,11 @@
/// file is up to date, but not otherwise.
bool IncludeTimestamps;
+ /// Indicates whether the AST file being written is an implicit module.
+ /// If that's the case, we may be able to skip writing some information that
+ /// are guaranteed to be the same in the importer by the context hash.
+ bool BuildingImplicitModule = false;
+
/// Indicates when the AST writing is actively performing
/// serialization, rather than just queueing updates.
bool WritingAST = false;
@@ -571,7 +576,7 @@
ASTWriter(llvm::BitstreamWriter &Stream, SmallVectorImpl<char> &Buffer,
InMemoryModuleCache &ModuleCache,
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
- bool IncludeTimestamps = true);
+ bool IncludeTimestamps = true, bool BuildingImplicitModule = false);
~ASTWriter() override;
ASTContext &getASTContext() const {
@@ -809,6 +814,7 @@
std::shared_ptr<PCHBuffer> Buffer,
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
bool AllowASTWithErrors = false, bool IncludeTimestamps = true,
+ bool BuildingImplicitModule = false,
bool ShouldCacheASTInMemory = false);
~PCHGenerator() override;
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -199,7 +199,7 @@
/// \returns true to indicate the preprocessor options are invalid, or false
/// otherwise.
virtual bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
- bool Complain,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) {
return false;
}
@@ -274,7 +274,7 @@
StringRef SpecificModuleCachePath,
bool Complain) override;
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
- bool Complain,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override;
void ReadCounter(const serialization::ModuleFile &M, unsigned Value) override;
@@ -304,7 +304,8 @@
bool AllowCompatibleDifferences) override;
bool ReadDiagnosticOptions(IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts,
bool Complain) override;
- bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain,
+ bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override;
bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
StringRef SpecificModuleCachePath,
@@ -325,7 +326,8 @@
public:
SimpleASTReaderListener(Preprocessor &PP) : PP(PP) {}
- bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain,
+ bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
+ bool ReadMacros, bool Complain,
std::string &SuggestedPredefines) override;
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
