[clang] 79f87be - [clang] Fix several issues in the generated AttrHasAttributeImpl.inc
Sergei Barannikov via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 9 23:04:09 PDT 2023
Author: Sergei Barannikov
Date: 2023-10-10T09:03:55+03:00
New Revision: 79f87be6888d13a94661be9d8908c83dd2229c9b
URL: https://github.com/llvm/llvm-project/commit/79f87be6888d13a94661be9d8908c83dd2229c9b
DIFF: https://github.com/llvm/llvm-project/commit/79f87be6888d13a94661be9d8908c83dd2229c9b.diff
LOG: [clang] Fix several issues in the generated AttrHasAttributeImpl.inc
1. The generated file contained a lot of duplicate switch cases, e.g.:
```
switch (Syntax) {
case AttributeCommonInfo::Syntax::AS_GNU:
return llvm::StringSwitch<int>(Name)
...
.Case("error", 1)
.Case("warning", 1)
.Case("error", 1)
.Case("warning", 1)
```
2. Some attributes were listed in wrong places, e.g.:
```
case AttributeCommonInfo::Syntax::AS_CXX11: {
if (ScopeName == "") {
return llvm::StringSwitch<int>(Name)
...
.Case("warn_unused_result", LangOpts.CPlusPlus11 ? 201907 : 0)
```
`warn_unused_result` is a non-standard attribute and should not be
available as [[warn_unused_result]].
3. Some attributes had the wrong version, e.g.:
```
case AttributeCommonInfo::Syntax::AS_CXX11: {
} else if (ScopeName == "gnu") {
return llvm::StringSwitch<int>(Name)
...
.Case("fallthrough", LangOpts.CPlusPlus11 ? 201603 : 0)
```
[[gnu::fallthrough]] is a non-standard spelling and should not have the
standard version. Instead, __has_cpp_attribute should return 1 for it.
There is another issue with attributes that share spellings, e.g.:
```
.Case("interrupt", true && (T.getArch() == llvm::Triple::arm || ...) ? 1 : 0)
.Case("interrupt", true && (T.getArch() == llvm::Triple::avr) ? 1 : 0)
...
.Case("interrupt", true && (T.getArch() == llvm::Triple::riscv32 || ...) ? 1 : 0)
```
As can be seen, __has_attribute(interrupt) would only return true for
ARM targets. This patch does not address this issue.
Differential Revision: https://reviews.llvm.org/D159393
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/test/Preprocessor/has_attribute.c
clang/test/Preprocessor/has_attribute.cpp
clang/test/Preprocessor/has_c_attribute.c
clang/utils/TableGen/ClangAttrEmitter.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d85db6f795c5274..c1db779cc86cb97 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -45,6 +45,24 @@ C/C++ Language Potentially Breaking Changes
-xc++-header``) is now ``.pch`` instead of ``.gch``.
- ``-include a.h`` probing ``a.h.gch`` is deprecated. Change the extension name
to ``.pch`` or use ``-include-pch a.h.gch``.
+- Fixed a bug that caused ``__has_cpp_attribute`` and ``__has_c_attribute``
+ return incorrect values for some C++-11-style attributes. Below is a complete
+ list of behavior changes.
+
+ .. csv-table::
+ :header: Test, Old value, New value
+
+ ``__has_cpp_attribute(unused)``, 201603, 0
+ ``__has_cpp_attribute(gnu::unused)``, 201603, 1
+ ``__has_c_attribute(unused)``, 202106, 0
+ ``__has_cpp_attribute(clang::fallthrough)``, 201603, 1
+ ``__has_cpp_attribute(gnu::fallthrough)``, 201603, 1
+ ``__has_c_attribute(gnu::fallthrough)``, 201910, 1
+ ``__has_cpp_attribute(warn_unused_result)``, 201907, 0
+ ``__has_cpp_attribute(clang::warn_unused_result)``, 201907, 1
+ ``__has_cpp_attribute(gnu::warn_unused_result)``, 201907, 1
+ ``__has_c_attribute(warn_unused_result)``, 202003, 0
+ ``__has_c_attribute(gnu::warn_unused_result)``, 202003, 1
C++ Specific Potentially Breaking Changes
-----------------------------------------
diff --git a/clang/test/Preprocessor/has_attribute.c b/clang/test/Preprocessor/has_attribute.c
index 77787c9b64edb53..0ba664a53e64962 100644
--- a/clang/test/Preprocessor/has_attribute.c
+++ b/clang/test/Preprocessor/has_attribute.c
@@ -10,6 +10,11 @@ int always_inline();
int __always_inline__();
#endif
+// CHECK: warn_unused_result
+#if __has_attribute(warn_unused_result)
+int warn_unused_result();
+#endif
+
// CHECK: no_dummy_attribute
#if !__has_attribute(dummy_attribute)
int no_dummy_attribute();
diff --git a/clang/test/Preprocessor/has_attribute.cpp b/clang/test/Preprocessor/has_attribute.cpp
index 3fb99eda699b3cf..33546dbb175f617 100644
--- a/clang/test/Preprocessor/has_attribute.cpp
+++ b/clang/test/Preprocessor/has_attribute.cpp
@@ -3,14 +3,14 @@
#define CXX11(x) x: __has_cpp_attribute(x)
-// CHECK: clang::fallthrough: 201603L
+// CHECK: clang::fallthrough: 1
CXX11(clang::fallthrough)
// CHECK: selectany: 0
CXX11(selectany)
// The attribute name can be bracketed with double underscores.
-// CHECK: clang::__fallthrough__: 201603L
+// CHECK: clang::__fallthrough__: 1
CXX11(clang::__fallthrough__)
// The scope cannot be bracketed with double underscores unless it is
@@ -18,12 +18,21 @@ CXX11(clang::__fallthrough__)
// CHECK: __gsl__::suppress: 0
CXX11(__gsl__::suppress)
-// CHECK: _Clang::fallthrough: 201603L
+// CHECK: _Clang::fallthrough: 1
CXX11(_Clang::fallthrough)
// CHECK: __nodiscard__: 201907L
CXX11(__nodiscard__)
+// CHECK: warn_unused_result: 0
+CXX11(warn_unused_result)
+
+// CHECK: gnu::warn_unused_result: 1
+CXX11(gnu::warn_unused_result)
+
+// CHECK: clang::warn_unused_result: 1
+CXX11(clang::warn_unused_result)
+
// CHECK: __gnu__::__const__: 1
CXX11(__gnu__::__const__)
diff --git a/clang/test/Preprocessor/has_c_attribute.c b/clang/test/Preprocessor/has_c_attribute.c
index 2f4fdf1679485f1..3332571d758c84f 100644
--- a/clang/test/Preprocessor/has_c_attribute.c
+++ b/clang/test/Preprocessor/has_c_attribute.c
@@ -9,6 +9,15 @@ C2x(fallthrough)
// CHECK: __nodiscard__: 202003L
C2x(__nodiscard__)
+// CHECK: warn_unused_result: 0
+C2x(warn_unused_result)
+
+// CHECK: gnu::warn_unused_result: 1
+C2x(gnu::warn_unused_result)
+
+// CHECK: clang::warn_unused_result: 0
+C2x(clang::warn_unused_result)
+
// CHECK: selectany: 0
C2x(selectany); // Known attribute not supported in C mode
@@ -27,10 +36,10 @@ C2x(deprecated)
// CHECK: maybe_unused: 202106L
C2x(maybe_unused)
-// CHECK: __gnu__::warn_unused_result: 202003L
+// CHECK: __gnu__::warn_unused_result: 1
C2x(__gnu__::warn_unused_result)
-// CHECK: gnu::__warn_unused_result__: 202003L
+// CHECK: gnu::__warn_unused_result__: 1
C2x(gnu::__warn_unused_result__)
// Test that macro expansion of the builtin argument works.
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index f2a6c1dfcf2fcc4..45e2fa7b283e18d 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3434,9 +3434,10 @@ static bool GenerateTargetSpecificAttrChecks(const Record *R,
}
static void GenerateHasAttrSpellingStringSwitch(
- const std::vector<Record *> &Attrs, raw_ostream &OS,
- const std::string &Variety = "", const std::string &Scope = "") {
- for (const auto *Attr : Attrs) {
+ const std::vector<std::pair<const Record *, FlattenedSpelling>> &Attrs,
+ raw_ostream &OS, const std::string &Variety,
+ const std::string &Scope = "") {
+ for (const auto &[Attr, Spelling] : Attrs) {
// C++11-style attributes have specific version information associated with
// them. If the attribute has no scope, the version information must not
// have the default value (1), as that's incorrect. Instead, the unscoped
@@ -3455,26 +3456,22 @@ static void GenerateHasAttrSpellingStringSwitch(
// a way that is impactful to the end user.
int Version = 1;
- std::vector<FlattenedSpelling> Spellings = GetFlattenedSpellings(*Attr);
+ assert(Spelling.variety() == Variety);
std::string Name = "";
- for (const auto &Spelling : Spellings) {
- if (Spelling.variety() == Variety &&
- (Spelling.nameSpace().empty() || Scope == Spelling.nameSpace())) {
- Name = Spelling.name();
- Version = static_cast<int>(
- Spelling.getSpellingRecord().getValueAsInt("Version"));
- // Verify that explicitly specified CXX11 and C23 spellings (i.e.
- // not inferred from Clang/GCC spellings) have a version that's
- //
diff erent than the default (1).
- bool RequiresValidVersion =
- (Variety == "CXX11" || Variety == "C23") &&
- Spelling.getSpellingRecord().getValueAsString("Variety") == Variety;
- if (RequiresValidVersion && Scope.empty() && Version == 1)
- PrintError(Spelling.getSpellingRecord().getLoc(),
- "Standard attributes must have "
- "valid version information.");
- break;
- }
+ if (Spelling.nameSpace().empty() || Scope == Spelling.nameSpace()) {
+ Name = Spelling.name();
+ Version = static_cast<int>(
+ Spelling.getSpellingRecord().getValueAsInt("Version"));
+ // Verify that explicitly specified CXX11 and C23 spellings (i.e.
+ // not inferred from Clang/GCC spellings) have a version that's
+ //
diff erent from the default (1).
+ bool RequiresValidVersion =
+ (Variety == "CXX11" || Variety == "C23") &&
+ Spelling.getSpellingRecord().getValueAsString("Variety") == Variety;
+ if (RequiresValidVersion && Scope.empty() && Version == 1)
+ PrintError(Spelling.getSpellingRecord().getLoc(),
+ "Standard attributes must have "
+ "valid version information.");
}
std::string Test;
@@ -3514,10 +3511,8 @@ static void GenerateHasAttrSpellingStringSwitch(
std::string TestStr = !Test.empty()
? Test + " ? " + llvm::itostr(Version) + " : 0"
: llvm::itostr(Version);
- for (const auto &S : Spellings)
- if (Variety.empty() || (Variety == S.variety() &&
- (Scope.empty() || Scope == S.nameSpace())))
- OS << " .Case(\"" << S.name() << "\", " << TestStr << ")\n";
+ if (Scope.empty() || Scope == Spelling.nameSpace())
+ OS << " .Case(\"" << Spelling.name() << "\", " << TestStr << ")\n";
}
OS << " .Default(0);\n";
}
@@ -3550,8 +3545,11 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
// Separate all of the attributes out into four group: generic, C++11, GNU,
// and declspecs. Then generate a big switch statement for each of them.
std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr");
- std::vector<Record *> Declspec, Microsoft, GNU, Pragma, HLSLSemantic;
- std::map<std::string, std::vector<Record *>> CXX, C23;
+ std::vector<std::pair<const Record *, FlattenedSpelling>> Declspec, Microsoft,
+ GNU, Pragma, HLSLSemantic;
+ std::map<std::string,
+ std::vector<std::pair<const Record *, FlattenedSpelling>>>
+ CXX, C23;
// Walk over the list of all attributes, and split them out based on the
// spelling variety.
@@ -3560,19 +3558,19 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
for (const auto &SI : Spellings) {
const std::string &Variety = SI.variety();
if (Variety == "GNU")
- GNU.push_back(R);
+ GNU.emplace_back(R, SI);
else if (Variety == "Declspec")
- Declspec.push_back(R);
+ Declspec.emplace_back(R, SI);
else if (Variety == "Microsoft")
- Microsoft.push_back(R);
+ Microsoft.emplace_back(R, SI);
else if (Variety == "CXX11")
- CXX[SI.nameSpace()].push_back(R);
+ CXX[SI.nameSpace()].emplace_back(R, SI);
else if (Variety == "C23")
- C23[SI.nameSpace()].push_back(R);
+ C23[SI.nameSpace()].emplace_back(R, SI);
else if (Variety == "Pragma")
- Pragma.push_back(R);
+ Pragma.emplace_back(R, SI);
else if (Variety == "HLSLSemantic")
- HLSLSemantic.push_back(R);
+ HLSLSemantic.emplace_back(R, SI);
}
}
@@ -3594,7 +3592,10 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
OS << " return llvm::StringSwitch<int>(Name)\n";
GenerateHasAttrSpellingStringSwitch(HLSLSemantic, OS, "HLSLSemantic");
auto fn = [&OS](const char *Spelling,
- const std::map<std::string, std::vector<Record *>> &List) {
+ const std::map<
+ std::string,
+ std::vector<std::pair<const Record *, FlattenedSpelling>>>
+ &List) {
OS << "case AttributeCommonInfo::Syntax::AS_" << Spelling << ": {\n";
// C++11-style attributes are further split out based on the Scope.
for (auto I = List.cbegin(), E = List.cend(); I != E; ++I) {
More information about the cfe-commits
mailing list