llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang <details> <summary>Changes</summary> `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`. --- Full diff: https://github.com/llvm/llvm-project/pull/67900.diff 4 Files Affected: - (modified) clang/include/clang/Basic/Module.h (+4-3) - (modified) clang/lib/Basic/Module.cpp (+5-5) - (modified) clang/lib/Lex/PPDirectives.cpp (+1-1) - (modified) clang/lib/Serialization/ASTWriter.cpp (+2-2) ``````````diff 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. `````````` </details> https://github.com/llvm/llvm-project/pull/67900 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
