[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[via cfe-commits]

https://github.com/cor3ntin closed 
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

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[via cfe-commits]

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 
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`. 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 MissingHeaders;
 
-  /// An individual requirement: a feature name and a flag indicating
-  /// the required state of that feature.
-  using Requirement = std::pair;
+  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 ,
   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 ,
 const TargetInfo ) {
-  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 , 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 ,
 // 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  : 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 
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`. 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 

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[via cfe-commits]

cor3ntin wrote:

@davidstone ping

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

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[Chuanqi Xu via cfe-commits]

ChuanqiXu9 wrote:

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

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

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[David Stone via cfe-commits]

davidstone wrote:

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

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

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[David Stone via cfe-commits]

https://github.com/davidstone updated 
https://github.com/llvm/llvm-project/pull/67900

>From b5e64ac8d36fef66cb8ef698f74997616d2957bc Mon Sep 17 00:00:00 2001
From: David Stone 
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`. 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 MissingHeaders;
 
-  /// An individual requirement: a feature name and a flag indicating
-  /// the required state of that feature.
-  using Requirement = std::pair;
+  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 ,
   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 ,
 const TargetInfo ) {
-  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 , 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 ,
 // 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  : 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 
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`. 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 

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[Chuanqi Xu via cfe-commits]

https://github.com/ChuanqiXu9 approved this pull request.

LGTM. Thanks.

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

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[David Blaikie via cfe-commits]

https://github.com/dwblaikie approved this pull request.

Awesome - love a good readability improvement. Thanks!

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

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[David Stone via cfe-commits]

davidstone wrote:

@ChuanqiXu9 

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

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[David Stone via cfe-commits]

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 
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`. 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 MissingHeaders;
 
-  /// An individual requirement: a feature name and a flag indicating
-  /// the required state of that feature.
-  using Requirement = std::pair;
+  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 ,
   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 ,
 const TargetInfo ) {
-  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 , 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 ,
 // 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  : 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 
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`. 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 

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[David Stone via cfe-commits]

https://github.com/davidstone updated 
https://github.com/llvm/llvm-project/pull/67900

>From 0253be550776631980d77e7847c6337786d64911 Mon Sep 17 00:00:00 2001
From: David Stone 
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`. 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 MissingHeaders;
 
-  /// An individual requirement: a feature name and a flag indicating
-  /// the required state of that feature.
-  using Requirement = std::pair;
+  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 ,
   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 ,
 const TargetInfo ) {
-  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 , 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 ,
 // 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  : 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 46e59890368f66ae4ba6b1d242d08c1a7ec7676a Mon Sep 17 00:00:00 2001
From: David Stone 
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`. 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 

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[via cfe-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff e7247f1010b546ca4d98a6ee40414d2d1cae0381 
0253be550776631980d77e7847c6337786d64911 -- clang/include/clang/Basic/Module.h 
clang/lib/Basic/Module.cpp clang/lib/Lex/PPDirectives.cpp 
clang/lib/Serialization/ASTWriter.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index fff0067bc..97511ce9a 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -141,7 +141,7 @@ bool Module::isUnimportable(const LangOptions ,
 }
 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 579836d47..c55ce4ce6 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -1915,7 +1915,8 @@ bool Preprocessor::checkModuleIsAvailable(const 
LangOptions ,
 // 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;
 }

``




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

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang


Changes

`Module::Requirement` was defined as a `std::pairstd::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 MissingHeaders;
 
-  /// An individual requirement: a feature name and a flag indicating
-  /// the required state of that feature.
-  using Requirement = std::pair;
+  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 ,
   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 ,
 const TargetInfo ) {
-  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 , 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 ,
 // 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  : 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.

``




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

[clang] [clang][Modules] Make `Module::Requirement` a struct (PR #67900)

[David Stone via cfe-commits]

https://github.com/davidstone created 
https://github.com/llvm/llvm-project/pull/67900

`Module::Requirement` was defined as a `std::pair`. 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`.

>From 0253be550776631980d77e7847c6337786d64911 Mon Sep 17 00:00:00 2001
From: David Stone 
Date: Sat, 30 Sep 2023 22:57:34 -0600
Subject: [PATCH] [clang][Modules] Make `Module::Requirement` a struct

`Module::Requirement` was defined as a `std::pair`. 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 MissingHeaders;
 
-  /// An individual requirement: a feature name and a flag indicating
-  /// the required state of that feature.
-  using Requirement = std::pair;
+  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 ,
   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 ,
 const TargetInfo ) {
-  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 , 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 ,
 // 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  : 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.

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits