[clang][Modules] Make Module::Requirement a struct

[davidstone]

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.

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Changes

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 --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.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[davidstone]

@ChuanqiXu9

[dwblaikie]

Awesome - love a good readability improvement. Thanks!

[ChuanqiXu9]

LGTM. Thanks.

[davidstone]

Would someone be able to merge this for me? I do not have permission.

[ChuanqiXu9]

Would someone be able to merge this for me? I do not have permission.

Sorry for not merging this in time. Could you rebase again since it has conflicts now.

[cor3ntin]

@davidstone ping