Skip to content

Commit 7a378d8

Browse files
authored
Merge pull request #7263 from apple/jan_svoboda/stable-20221013-cmdline-macros-in-pcm-cherry-pick
[clang][modules] Cherry-pick command-line macro optimization
2 parents ac022a0 + ea36041 commit 7a378d8

File tree

8 files changed

+127
-101
lines changed

8 files changed

+127
-101
lines changed

clang/include/clang/Serialization/ASTReader.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ class ASTReaderListener {
199199
/// \returns true to indicate the preprocessor options are invalid, or false
200200
/// otherwise.
201201
virtual bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
202-
bool Complain,
202+
bool ReadMacros, bool Complain,
203203
std::string &SuggestedPredefines) {
204204
return false;
205205
}
@@ -294,7 +294,7 @@ class ChainedASTReaderListener : public ASTReaderListener {
294294
StringRef SpecificModuleCachePath,
295295
bool Complain) override;
296296
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
297-
bool Complain,
297+
bool ReadMacros, bool Complain,
298298
std::string &SuggestedPredefines) override;
299299

300300
void ReadCounter(const serialization::ModuleFile &M, unsigned Value) override;
@@ -328,7 +328,8 @@ class PCHValidator : public ASTReaderListener {
328328
bool AllowCompatibleDifferences) override;
329329
bool ReadDiagnosticOptions(IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts,
330330
bool Complain) override;
331-
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain,
331+
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
332+
bool ReadMacros, bool Complain,
332333
std::string &SuggestedPredefines) override;
333334
bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
334335
StringRef SpecificModuleCachePath,
@@ -349,7 +350,8 @@ class SimpleASTReaderListener : public ASTReaderListener {
349350
public:
350351
SimpleASTReaderListener(Preprocessor &PP) : PP(PP) {}
351352

352-
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain,
353+
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
354+
bool ReadMacros, bool Complain,
353355
std::string &SuggestedPredefines) override;
354356
};
355357

clang/include/clang/Serialization/ASTWriter.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,11 @@ class ASTWriter : public ASTDeserializationListener,
143143
/// file is up to date, but not otherwise.
144144
bool IncludeTimestamps;
145145

146+
/// Indicates whether the AST file being written is an implicit module.
147+
/// If that's the case, we may be able to skip writing some information that
148+
/// are guaranteed to be the same in the importer by the context hash.
149+
bool BuildingImplicitModule = false;
150+
146151
/// Indicates when the AST writing is actively performing
147152
/// serialization, rather than just queueing updates.
148153
bool WritingAST = false;
@@ -571,7 +576,7 @@ class ASTWriter : public ASTDeserializationListener,
571576
ASTWriter(llvm::BitstreamWriter &Stream, SmallVectorImpl<char> &Buffer,
572577
InMemoryModuleCache &ModuleCache,
573578
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
574-
bool IncludeTimestamps = true);
579+
bool IncludeTimestamps = true, bool BuildingImplicitModule = false);
575580
~ASTWriter() override;
576581

577582
ASTContext &getASTContext() const {
@@ -809,6 +814,7 @@ class PCHGenerator : public SemaConsumer {
809814
std::shared_ptr<PCHBuffer> Buffer,
810815
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
811816
bool AllowASTWithErrors = false, bool IncludeTimestamps = true,
817+
bool BuildingImplicitModule = false,
812818
bool ShouldCacheASTInMemory = false);
813819
~PCHGenerator() override;
814820

clang/lib/Frontend/ASTUnit.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,8 @@ class ASTInfoCollector : public ASTReaderListener {
586586
return false;
587587
}
588588

589-
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain,
589+
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
590+
bool ReadMacros, bool Complain,
590591
std::string &SuggestedPredefines) override {
591592
this->PPOpts = PPOpts;
592593
return false;

clang/lib/Frontend/FrontendActions.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ GeneratePCHAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
140140
CI.getPreprocessor(), CI.getModuleCache(), OutputFile, Sysroot, Buffer,
141141
FrontendOpts.ModuleFileExtensions,
142142
CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
143-
FrontendOpts.IncludeTimestamps, +CI.getLangOpts().CacheGeneratedPCH));
143+
FrontendOpts.IncludeTimestamps, FrontendOpts.BuildingImplicitModule,
144+
+CI.getLangOpts().CacheGeneratedPCH));
144145
Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
145146
CI, std::string(InFile), OutputFile, std::move(OS), Buffer));
146147

@@ -202,6 +203,7 @@ GenerateModuleAction::CreateASTConsumer(CompilerInstance &CI,
202203
+CI.getFrontendOpts().AllowPCMWithCompilerErrors,
203204
/*IncludeTimestamps=*/
204205
+CI.getFrontendOpts().BuildingImplicitModule,
206+
/*BuildingImplicitModule=*/+CI.getFrontendOpts().BuildingImplicitModule,
205207
/*ShouldCacheASTInMemory=*/
206208
+CI.getFrontendOpts().BuildingImplicitModule));
207209
Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
@@ -728,15 +730,15 @@ namespace {
728730
}
729731

730732
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
731-
bool Complain,
733+
bool ReadMacros, bool Complain,
732734
std::string &SuggestedPredefines) override {
733735
Out.indent(2) << "Preprocessor options:\n";
734736
DUMP_BOOLEAN(PPOpts.UsePredefines,
735737
"Uses compiler/target-specific predefines [-undef]");
736738
DUMP_BOOLEAN(PPOpts.DetailedRecord,
737739
"Uses detailed preprocessing record (for indexing)");
738740

739-
if (!PPOpts.Macros.empty()) {
741+
if (ReadMacros) {
740742
Out.indent(4) << "Predefined macros:\n";
741743
}
742744

clang/lib/Serialization/ASTReader.cpp

Lines changed: 78 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,12 @@ bool ChainedASTReaderListener::ReadHeaderSearchOptions(
208208
}
209209

210210
bool ChainedASTReaderListener::ReadPreprocessorOptions(
211-
const PreprocessorOptions &PPOpts, bool Complain,
211+
const PreprocessorOptions &PPOpts, bool ReadMacros, bool Complain,
212212
std::string &SuggestedPredefines) {
213-
return First->ReadPreprocessorOptions(PPOpts, Complain,
213+
return First->ReadPreprocessorOptions(PPOpts, ReadMacros, Complain,
214214
SuggestedPredefines) ||
215-
Second->ReadPreprocessorOptions(PPOpts, Complain, SuggestedPredefines);
215+
Second->ReadPreprocessorOptions(PPOpts, ReadMacros, Complain,
216+
SuggestedPredefines);
216217
}
217218

218219
void ChainedASTReaderListener::ReadCounter(const serialization::ModuleFile &M,
@@ -668,68 +669,70 @@ enum OptionValidation {
668669
/// are no differences in the options between the two.
669670
static bool checkPreprocessorOptions(
670671
const PreprocessorOptions &PPOpts,
671-
const PreprocessorOptions &ExistingPPOpts, DiagnosticsEngine *Diags,
672-
FileManager &FileMgr, std::string &SuggestedPredefines,
673-
const LangOptions &LangOpts,
672+
const PreprocessorOptions &ExistingPPOpts, bool ReadMacros,
673+
DiagnosticsEngine *Diags, FileManager &FileMgr,
674+
std::string &SuggestedPredefines, const LangOptions &LangOpts,
674675
OptionValidation Validation = OptionValidateContradictions) {
675-
// Check macro definitions.
676-
MacroDefinitionsMap ASTFileMacros;
677-
collectMacroDefinitions(PPOpts, ASTFileMacros);
678-
MacroDefinitionsMap ExistingMacros;
679-
SmallVector<StringRef, 4> ExistingMacroNames;
680-
collectMacroDefinitions(ExistingPPOpts, ExistingMacros, &ExistingMacroNames);
681-
682-
for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) {
676+
if (ReadMacros) {
677+
// Check macro definitions.
678+
MacroDefinitionsMap ASTFileMacros;
679+
collectMacroDefinitions(PPOpts, ASTFileMacros);
680+
MacroDefinitionsMap ExistingMacros;
681+
SmallVector<StringRef, 4> ExistingMacroNames;
682+
collectMacroDefinitions(ExistingPPOpts, ExistingMacros,
683+
&ExistingMacroNames);
684+
685+
for (unsigned I = 0, N = ExistingMacroNames.size(); I != N; ++I) {
683686
// Dig out the macro definition in the existing preprocessor options.
684687
StringRef MacroName = ExistingMacroNames[I];
685688
std::pair<StringRef, bool> Existing = ExistingMacros[MacroName];
686689

687-
// Check whether we know anything about this macro name or not.
688-
llvm::StringMap<std::pair<StringRef, bool /*IsUndef*/>>::iterator Known =
689-
ASTFileMacros.find(MacroName);
690-
if (Validation == OptionValidateNone || Known == ASTFileMacros.end()) {
691-
if (Validation == OptionValidateStrictMatches) {
692-
// If strict matches are requested, don't tolerate any extra defines on
693-
// the command line that are missing in the AST file.
690+
// Check whether we know anything about this macro name or not.
691+
llvm::StringMap<std::pair<StringRef, bool /*IsUndef*/>>::iterator Known =
692+
ASTFileMacros.find(MacroName);
693+
if (Validation == OptionValidateNone || Known == ASTFileMacros.end()) {
694+
if (Validation == OptionValidateStrictMatches) {
695+
// If strict matches are requested, don't tolerate any extra defines
696+
// on the command line that are missing in the AST file.
697+
if (Diags) {
698+
Diags->Report(diag::err_pch_macro_def_undef) << MacroName << true;
699+
}
700+
return true;
701+
}
702+
// FIXME: Check whether this identifier was referenced anywhere in the
703+
// AST file. If so, we should reject the AST file. Unfortunately, this
704+
// information isn't in the control block. What shall we do about it?
705+
706+
if (Existing.second) {
707+
SuggestedPredefines += "#undef ";
708+
SuggestedPredefines += MacroName.str();
709+
SuggestedPredefines += '\n';
710+
} else {
711+
SuggestedPredefines += "#define ";
712+
SuggestedPredefines += MacroName.str();
713+
SuggestedPredefines += ' ';
714+
SuggestedPredefines += Existing.first.str();
715+
SuggestedPredefines += '\n';
716+
}
717+
continue;
718+
}
719+
720+
// If the macro was defined in one but undef'd in the other, we have a
721+
// conflict.
722+
if (Existing.second != Known->second.second) {
694723
if (Diags) {
695-
Diags->Report(diag::err_pch_macro_def_undef) << MacroName << true;
724+
Diags->Report(diag::err_pch_macro_def_undef)
725+
<< MacroName << Known->second.second;
696726
}
697727
return true;
698728
}
699-
// FIXME: Check whether this identifier was referenced anywhere in the
700-
// AST file. If so, we should reject the AST file. Unfortunately, this
701-
// information isn't in the control block. What shall we do about it?
702-
703-
if (Existing.second) {
704-
SuggestedPredefines += "#undef ";
705-
SuggestedPredefines += MacroName.str();
706-
SuggestedPredefines += '\n';
707-
} else {
708-
SuggestedPredefines += "#define ";
709-
SuggestedPredefines += MacroName.str();
710-
SuggestedPredefines += ' ';
711-
SuggestedPredefines += Existing.first.str();
712-
SuggestedPredefines += '\n';
713-
}
714-
continue;
715-
}
716729

717-
// If the macro was defined in one but undef'd in the other, we have a
718-
// conflict.
719-
if (Existing.second != Known->second.second) {
720-
if (Diags) {
721-
Diags->Report(diag::err_pch_macro_def_undef)
722-
<< MacroName << Known->second.second;
730+
// If the macro was #undef'd in both, or if the macro bodies are
731+
// identical, it's fine.
732+
if (Existing.second || Existing.first == Known->second.first) {
733+
ASTFileMacros.erase(Known);
734+
continue;
723735
}
724-
return true;
725-
}
726-
727-
// If the macro was #undef'd in both, or if the macro bodies are identical,
728-
// it's fine.
729-
if (Existing.second || Existing.first == Known->second.first) {
730-
ASTFileMacros.erase(Known);
731-
continue;
732-
}
733736

734737
// The macro bodies differ; complain.
735738
if (Diags) {
@@ -745,7 +748,7 @@ static bool checkPreprocessorOptions(
745748
if (Diags) {
746749
Diags->Report(diag::err_pch_macro_def_undef) << MacroName << false;
747750
}
748-
return true;
751+
return true;}
749752
}
750753
}
751754

@@ -807,24 +810,22 @@ static bool checkPreprocessorOptions(
807810
}
808811

809812
bool PCHValidator::ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
810-
bool Complain,
813+
bool ReadMacros, bool Complain,
811814
std::string &SuggestedPredefines) {
812815
const PreprocessorOptions &ExistingPPOpts = PP.getPreprocessorOpts();
813816

814-
return checkPreprocessorOptions(PPOpts, ExistingPPOpts,
815-
Complain? &Reader.Diags : nullptr,
816-
PP.getFileManager(),
817-
SuggestedPredefines,
818-
PP.getLangOpts());
817+
return checkPreprocessorOptions(
818+
PPOpts, ExistingPPOpts, ReadMacros, Complain ? &Reader.Diags : nullptr,
819+
PP.getFileManager(), SuggestedPredefines, PP.getLangOpts());
819820
}
820821

821822
bool SimpleASTReaderListener::ReadPreprocessorOptions(
822-
const PreprocessorOptions &PPOpts,
823-
bool Complain,
824-
std::string &SuggestedPredefines) {
825-
return checkPreprocessorOptions(PPOpts, PP.getPreprocessorOpts(), nullptr,
826-
PP.getFileManager(), SuggestedPredefines,
827-
PP.getLangOpts(), OptionValidateNone);
823+
const PreprocessorOptions &PPOpts, bool ReadMacros, bool Complain,
824+
std::string &SuggestedPredefines) {
825+
return checkPreprocessorOptions(PPOpts, PP.getPreprocessorOpts(), ReadMacros,
826+
nullptr, PP.getFileManager(),
827+
SuggestedPredefines, PP.getLangOpts(),
828+
OptionValidateNone);
828829
}
829830

830831
/// Check the header search options deserialized from the control block
@@ -5234,10 +5235,10 @@ namespace {
52345235
}
52355236

52365237
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
5237-
bool Complain,
5238+
bool ReadMacros, bool Complain,
52385239
std::string &SuggestedPredefines) override {
52395240
return checkPreprocessorOptions(
5240-
PPOpts, ExistingPPOpts, /*Diags=*/nullptr, FileMgr,
5241+
PPOpts, ExistingPPOpts, ReadMacros, /*Diags=*/nullptr, FileMgr,
52415242
SuggestedPredefines, ExistingLangOpts,
52425243
StrictOptionMatches ? OptionValidateStrictMatches
52435244
: OptionValidateContradictions);
@@ -6018,10 +6019,13 @@ bool ASTReader::ParsePreprocessorOptions(const RecordData &Record,
60186019
unsigned Idx = 0;
60196020

60206021
// Macro definitions/undefs
6021-
for (unsigned N = Record[Idx++]; N; --N) {
6022-
std::string Macro = ReadString(Record, Idx);
6023-
bool IsUndef = Record[Idx++];
6024-
PPOpts.Macros.push_back(std::make_pair(Macro, IsUndef));
6022+
bool ReadMacros = Record[Idx++];
6023+
if (ReadMacros) {
6024+
for (unsigned N = Record[Idx++]; N; --N) {
6025+
std::string Macro = ReadString(Record, Idx);
6026+
bool IsUndef = Record[Idx++];
6027+
PPOpts.Macros.push_back(std::make_pair(Macro, IsUndef));
6028+
}
60256029
}
60266030

60276031
// Includes
@@ -6040,7 +6044,7 @@ bool ASTReader::ParsePreprocessorOptions(const RecordData &Record,
60406044
PPOpts.ObjCXXARCStandardLibrary =
60416045
static_cast<ObjCXXARCStandardLibraryKind>(Record[Idx++]);
60426046
SuggestedPredefines.clear();
6043-
return Listener.ReadPreprocessorOptions(PPOpts, Complain,
6047+
return Listener.ReadPreprocessorOptions(PPOpts, ReadMacros, Complain,
60446048
SuggestedPredefines);
60456049
}
60466050

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1497,11 +1497,19 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
14971497
Record.clear();
14981498
const PreprocessorOptions &PPOpts = PP.getPreprocessorOpts();
14991499

1500-
// Macro definitions.
1501-
Record.push_back(PPOpts.Macros.size());
1502-
for (unsigned I = 0, N = PPOpts.Macros.size(); I != N; ++I) {
1503-
AddString(PPOpts.Macros[I].first, Record);
1504-
Record.push_back(PPOpts.Macros[I].second);
1500+
// If we're building an implicit module with a context hash, the importer is
1501+
// guaranteed to have the same macros defined on the command line. Skip
1502+
// writing them.
1503+
bool SkipMacros = BuildingImplicitModule && !HSOpts.DisableModuleHash;
1504+
bool WriteMacros = !SkipMacros;
1505+
Record.push_back(WriteMacros);
1506+
if (WriteMacros) {
1507+
// Macro definitions.
1508+
Record.push_back(PPOpts.Macros.size());
1509+
for (unsigned I = 0, N = PPOpts.Macros.size(); I != N; ++I) {
1510+
AddString(PPOpts.Macros[I].first, Record);
1511+
Record.push_back(PPOpts.Macros[I].second);
1512+
}
15051513
}
15061514

15071515
// Includes
@@ -4506,9 +4514,10 @@ ASTWriter::ASTWriter(llvm::BitstreamWriter &Stream,
45064514
SmallVectorImpl<char> &Buffer,
45074515
InMemoryModuleCache &ModuleCache,
45084516
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
4509-
bool IncludeTimestamps)
4517+
bool IncludeTimestamps, bool BuildingImplicitModule)
45104518
: Stream(Stream), Buffer(Buffer), ModuleCache(ModuleCache),
4511-
IncludeTimestamps(IncludeTimestamps) {
4519+
IncludeTimestamps(IncludeTimestamps),
4520+
BuildingImplicitModule(BuildingImplicitModule) {
45124521
for (const auto &Ext : Extensions) {
45134522
if (auto Writer = Ext->createExtensionWriter(*this))
45144523
ModuleFileExtensionWriters.push_back(std::move(Writer));

clang/lib/Serialization/GeneratePCH.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ PCHGenerator::PCHGenerator(
2525
StringRef OutputFile, StringRef isysroot, std::shared_ptr<PCHBuffer> Buffer,
2626
ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
2727
bool AllowASTWithErrors, bool IncludeTimestamps,
28-
bool ShouldCacheASTInMemory)
28+
bool BuildingImplicitModule, bool ShouldCacheASTInMemory)
2929
: PP(PP), OutputFile(OutputFile), isysroot(isysroot.str()),
3030
SemaPtr(nullptr), Buffer(std::move(Buffer)), Stream(this->Buffer->Data),
3131
Writer(Stream, this->Buffer->Data, ModuleCache, Extensions,
32-
IncludeTimestamps),
32+
IncludeTimestamps, BuildingImplicitModule),
3333
AllowASTWithErrors(AllowASTWithErrors),
3434
ShouldCacheASTInMemory(ShouldCacheASTInMemory) {
3535
this->Buffer->IsComplete = false;

0 commit comments

Comments
 (0)