https://github.com/davidstone updated https://github.com/llvm/llvm-project/pull/67900
>From 858a76e16a772078b29452645c5beb223c06dc8e Mon Sep 17 00:00:00 2001 From: David Stone <davidfromonl...@gmail.com> Date: Sat, 30 Sep 2023 22:57:34 -0600 Subject: [PATCH 1/2] [clang][Modules] Make `Module::Requirement` a struct `Module::Requirement` was defined as a `std::pair<std::string, bool>`. This required a comment to explain what the data members mean and makes the usage harder to understand. Replace this with a struct with two members, `FeatureName` and `RequiredState`. --- clang/include/clang/Basic/Module.h | 7 ++++--- clang/lib/Basic/Module.cpp | 10 +++++----- clang/lib/Lex/PPDirectives.cpp | 2 +- clang/lib/Serialization/ASTWriter.cpp | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 676fd372493a3aa..0a134b53d3d9801 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -281,9 +281,10 @@ class alignas(8) Module { /// found on the file system. SmallVector<UnresolvedHeaderDirective, 1> MissingHeaders; - /// An individual requirement: a feature name and a flag indicating - /// the required state of that feature. - using Requirement = std::pair<std::string, bool>; + struct Requirement { + std::string FeatureName; + bool RequiredState; + }; /// The set of language features required to use this module. /// diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index 0455304ef7f2b1a..fff0067bc9f818f 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -140,8 +140,8 @@ bool Module::isUnimportable(const LangOptions &LangOpts, return true; } for (unsigned I = 0, N = Current->Requirements.size(); I != N; ++I) { - if (hasFeature(Current->Requirements[I].first, LangOpts, Target) != - Current->Requirements[I].second) { + if (hasFeature(Current->Requirements[I].FeatureName, LangOpts, Target) != + Current->Requirements[I].RequiredState) { Req = Current->Requirements[I]; return true; } @@ -312,7 +312,7 @@ bool Module::directlyUses(const Module *Requested) { void Module::addRequirement(StringRef Feature, bool RequiredState, const LangOptions &LangOpts, const TargetInfo &Target) { - Requirements.push_back(Requirement(std::string(Feature), RequiredState)); + Requirements.push_back(Requirement{std::string(Feature), RequiredState}); // If this feature is currently available, we're done. if (hasFeature(Feature, LangOpts, Target) == RequiredState) @@ -497,9 +497,9 @@ void Module::print(raw_ostream &OS, unsigned Indent, bool Dump) const { for (unsigned I = 0, N = Requirements.size(); I != N; ++I) { if (I) OS << ", "; - if (!Requirements[I].second) + if (!Requirements[I].RequiredState) OS << "!"; - OS << Requirements[I].first; + OS << Requirements[I].FeatureName; } OS << "\n"; } diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 7899bfa1c4f5842..579836d47201355 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -1915,7 +1915,7 @@ bool Preprocessor::checkModuleIsAvailable(const LangOptions &LangOpts, // FIXME: Track the location at which the requirement was specified, and // use it here. Diags.Report(M->DefinitionLoc, diag::err_module_unavailable) - << M->getFullModuleName() << Requirement.second << Requirement.first; + << M->getFullModuleName() << Requirement.RequiredState << Requirement.FeatureName; } return true; } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 201e2fcaaec91aa..f03b47ec3214d1b 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -2896,8 +2896,8 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) { // Emit the requirements. for (const auto &R : Mod->Requirements) { - RecordData::value_type Record[] = {SUBMODULE_REQUIRES, R.second}; - Stream.EmitRecordWithBlob(RequiresAbbrev, Record, R.first); + RecordData::value_type Record[] = {SUBMODULE_REQUIRES, R.RequiredState}; + Stream.EmitRecordWithBlob(RequiresAbbrev, Record, R.FeatureName); } // Emit the umbrella header, if there is one. >From 6ca9fe84266c9d4257ba0102c571f4f4771f8bb2 Mon Sep 17 00:00:00 2001 From: David Stone <davidfromonl...@gmail.com> Date: Sat, 30 Sep 2023 23:08:58 -0600 Subject: [PATCH 2/2] amend! [clang][Modules] Make `Module::Requirement` a struct [clang][Modules] Make `Module::Requirement` a struct `Module::Requirement` was defined as a `std::pair<std::string, bool>`. This required a comment to explain what the data members mean and makes the usage harder to understand. Replace this with a struct with two members, `FeatureName` and `RequiredState`. --- clang/lib/Basic/Module.cpp | 2 +- clang/lib/Lex/PPDirectives.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index fff0067bc9f818f..97511ce9a417315 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -141,7 +141,7 @@ bool Module::isUnimportable(const LangOptions &LangOpts, } for (unsigned I = 0, N = Current->Requirements.size(); I != N; ++I) { if (hasFeature(Current->Requirements[I].FeatureName, LangOpts, Target) != - Current->Requirements[I].RequiredState) { + Current->Requirements[I].RequiredState) { Req = Current->Requirements[I]; return true; } diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 579836d47201355..c55ce4ce6db975f 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -1915,7 +1915,8 @@ bool Preprocessor::checkModuleIsAvailable(const LangOptions &LangOpts, // FIXME: Track the location at which the requirement was specified, and // use it here. Diags.Report(M->DefinitionLoc, diag::err_module_unavailable) - << M->getFullModuleName() << Requirement.RequiredState << Requirement.FeatureName; + << M->getFullModuleName() << Requirement.RequiredState + << Requirement.FeatureName; } return true; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits