[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)
via cfe-commits
cfe-commits at lists.llvm.org
Fri May 3 01:10:03 PDT 2024
https://github.com/cor3ntin updated https://github.com/llvm/llvm-project/pull/67900
>From b5e64ac8d36fef66cb8ef698f74997616d2957bc Mon Sep 17 00:00:00 2001
From: David Stone <davidfromonline at 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 676fd372493a3a..0a134b53d3d980 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 0455304ef7f2b1..fff0067bc9f818 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 7899bfa1c4f584..579836d4720135 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 201e2fcaaec91a..f03b47ec3214d1 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 c74e03c6c3df6aef31dc343a3732d5327b37338f Mon Sep 17 00:00:00 2001
From: David Stone <davidfromonline at 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 fff0067bc9f818..97511ce9a41731 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 579836d4720135..c55ce4ce6db975 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;
}
More information about the cfe-commits
mailing list