Skip to content

Commit 79f87be

Browse files
committed
[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
1 parent 7926744 commit 79f87be

File tree

5 files changed

+83
-41
lines changed

5 files changed

+83
-41
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,24 @@ C/C++ Language Potentially Breaking Changes
4545
-xc++-header``) is now ``.pch`` instead of ``.gch``.
4646
- ``-include a.h`` probing ``a.h.gch`` is deprecated. Change the extension name
4747
to ``.pch`` or use ``-include-pch a.h.gch``.
48+
- Fixed a bug that caused ``__has_cpp_attribute`` and ``__has_c_attribute``
49+
return incorrect values for some C++-11-style attributes. Below is a complete
50+
list of behavior changes.
51+
52+
.. csv-table::
53+
:header: Test, Old value, New value
54+
55+
``__has_cpp_attribute(unused)``, 201603, 0
56+
``__has_cpp_attribute(gnu::unused)``, 201603, 1
57+
``__has_c_attribute(unused)``, 202106, 0
58+
``__has_cpp_attribute(clang::fallthrough)``, 201603, 1
59+
``__has_cpp_attribute(gnu::fallthrough)``, 201603, 1
60+
``__has_c_attribute(gnu::fallthrough)``, 201910, 1
61+
``__has_cpp_attribute(warn_unused_result)``, 201907, 0
62+
``__has_cpp_attribute(clang::warn_unused_result)``, 201907, 1
63+
``__has_cpp_attribute(gnu::warn_unused_result)``, 201907, 1
64+
``__has_c_attribute(warn_unused_result)``, 202003, 0
65+
``__has_c_attribute(gnu::warn_unused_result)``, 202003, 1
4866

4967
C++ Specific Potentially Breaking Changes
5068
-----------------------------------------

clang/test/Preprocessor/has_attribute.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ int always_inline();
1010
int __always_inline__();
1111
#endif
1212

13+
// CHECK: warn_unused_result
14+
#if __has_attribute(warn_unused_result)
15+
int warn_unused_result();
16+
#endif
17+
1318
// CHECK: no_dummy_attribute
1419
#if !__has_attribute(dummy_attribute)
1520
int no_dummy_attribute();

clang/test/Preprocessor/has_attribute.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,36 @@
33

44
#define CXX11(x) x: __has_cpp_attribute(x)
55

6-
// CHECK: clang::fallthrough: 201603L
6+
// CHECK: clang::fallthrough: 1
77
CXX11(clang::fallthrough)
88

99
// CHECK: selectany: 0
1010
CXX11(selectany)
1111

1212
// The attribute name can be bracketed with double underscores.
13-
// CHECK: clang::__fallthrough__: 201603L
13+
// CHECK: clang::__fallthrough__: 1
1414
CXX11(clang::__fallthrough__)
1515

1616
// The scope cannot be bracketed with double underscores unless it is
1717
// for gnu or clang.
1818
// CHECK: __gsl__::suppress: 0
1919
CXX11(__gsl__::suppress)
2020

21-
// CHECK: _Clang::fallthrough: 201603L
21+
// CHECK: _Clang::fallthrough: 1
2222
CXX11(_Clang::fallthrough)
2323

2424
// CHECK: __nodiscard__: 201907L
2525
CXX11(__nodiscard__)
2626

27+
// CHECK: warn_unused_result: 0
28+
CXX11(warn_unused_result)
29+
30+
// CHECK: gnu::warn_unused_result: 1
31+
CXX11(gnu::warn_unused_result)
32+
33+
// CHECK: clang::warn_unused_result: 1
34+
CXX11(clang::warn_unused_result)
35+
2736
// CHECK: __gnu__::__const__: 1
2837
CXX11(__gnu__::__const__)
2938

clang/test/Preprocessor/has_c_attribute.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@ C2x(fallthrough)
99
// CHECK: __nodiscard__: 202003L
1010
C2x(__nodiscard__)
1111

12+
// CHECK: warn_unused_result: 0
13+
C2x(warn_unused_result)
14+
15+
// CHECK: gnu::warn_unused_result: 1
16+
C2x(gnu::warn_unused_result)
17+
18+
// CHECK: clang::warn_unused_result: 0
19+
C2x(clang::warn_unused_result)
20+
1221
// CHECK: selectany: 0
1322
C2x(selectany); // Known attribute not supported in C mode
1423

@@ -27,10 +36,10 @@ C2x(deprecated)
2736
// CHECK: maybe_unused: 202106L
2837
C2x(maybe_unused)
2938

30-
// CHECK: __gnu__::warn_unused_result: 202003L
39+
// CHECK: __gnu__::warn_unused_result: 1
3140
C2x(__gnu__::warn_unused_result)
3241

33-
// CHECK: gnu::__warn_unused_result__: 202003L
42+
// CHECK: gnu::__warn_unused_result__: 1
3443
C2x(gnu::__warn_unused_result__)
3544

3645
// Test that macro expansion of the builtin argument works.

clang/utils/TableGen/ClangAttrEmitter.cpp

Lines changed: 37 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3434,9 +3434,10 @@ static bool GenerateTargetSpecificAttrChecks(const Record *R,
34343434
}
34353435

34363436
static void GenerateHasAttrSpellingStringSwitch(
3437-
const std::vector<Record *> &Attrs, raw_ostream &OS,
3438-
const std::string &Variety = "", const std::string &Scope = "") {
3439-
for (const auto *Attr : Attrs) {
3437+
const std::vector<std::pair<const Record *, FlattenedSpelling>> &Attrs,
3438+
raw_ostream &OS, const std::string &Variety,
3439+
const std::string &Scope = "") {
3440+
for (const auto &[Attr, Spelling] : Attrs) {
34403441
// C++11-style attributes have specific version information associated with
34413442
// them. If the attribute has no scope, the version information must not
34423443
// have the default value (1), as that's incorrect. Instead, the unscoped
@@ -3455,26 +3456,22 @@ static void GenerateHasAttrSpellingStringSwitch(
34553456
// a way that is impactful to the end user.
34563457
int Version = 1;
34573458

3458-
std::vector<FlattenedSpelling> Spellings = GetFlattenedSpellings(*Attr);
3459+
assert(Spelling.variety() == Variety);
34593460
std::string Name = "";
3460-
for (const auto &Spelling : Spellings) {
3461-
if (Spelling.variety() == Variety &&
3462-
(Spelling.nameSpace().empty() || Scope == Spelling.nameSpace())) {
3463-
Name = Spelling.name();
3464-
Version = static_cast<int>(
3465-
Spelling.getSpellingRecord().getValueAsInt("Version"));
3466-
// Verify that explicitly specified CXX11 and C23 spellings (i.e.
3467-
// not inferred from Clang/GCC spellings) have a version that's
3468-
// different than the default (1).
3469-
bool RequiresValidVersion =
3470-
(Variety == "CXX11" || Variety == "C23") &&
3471-
Spelling.getSpellingRecord().getValueAsString("Variety") == Variety;
3472-
if (RequiresValidVersion && Scope.empty() && Version == 1)
3473-
PrintError(Spelling.getSpellingRecord().getLoc(),
3474-
"Standard attributes must have "
3475-
"valid version information.");
3476-
break;
3477-
}
3461+
if (Spelling.nameSpace().empty() || Scope == Spelling.nameSpace()) {
3462+
Name = Spelling.name();
3463+
Version = static_cast<int>(
3464+
Spelling.getSpellingRecord().getValueAsInt("Version"));
3465+
// Verify that explicitly specified CXX11 and C23 spellings (i.e.
3466+
// not inferred from Clang/GCC spellings) have a version that's
3467+
// different from the default (1).
3468+
bool RequiresValidVersion =
3469+
(Variety == "CXX11" || Variety == "C23") &&
3470+
Spelling.getSpellingRecord().getValueAsString("Variety") == Variety;
3471+
if (RequiresValidVersion && Scope.empty() && Version == 1)
3472+
PrintError(Spelling.getSpellingRecord().getLoc(),
3473+
"Standard attributes must have "
3474+
"valid version information.");
34783475
}
34793476

34803477
std::string Test;
@@ -3514,10 +3511,8 @@ static void GenerateHasAttrSpellingStringSwitch(
35143511
std::string TestStr = !Test.empty()
35153512
? Test + " ? " + llvm::itostr(Version) + " : 0"
35163513
: llvm::itostr(Version);
3517-
for (const auto &S : Spellings)
3518-
if (Variety.empty() || (Variety == S.variety() &&
3519-
(Scope.empty() || Scope == S.nameSpace())))
3520-
OS << " .Case(\"" << S.name() << "\", " << TestStr << ")\n";
3514+
if (Scope.empty() || Scope == Spelling.nameSpace())
3515+
OS << " .Case(\"" << Spelling.name() << "\", " << TestStr << ")\n";
35213516
}
35223517
OS << " .Default(0);\n";
35233518
}
@@ -3550,8 +3545,11 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
35503545
// Separate all of the attributes out into four group: generic, C++11, GNU,
35513546
// and declspecs. Then generate a big switch statement for each of them.
35523547
std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr");
3553-
std::vector<Record *> Declspec, Microsoft, GNU, Pragma, HLSLSemantic;
3554-
std::map<std::string, std::vector<Record *>> CXX, C23;
3548+
std::vector<std::pair<const Record *, FlattenedSpelling>> Declspec, Microsoft,
3549+
GNU, Pragma, HLSLSemantic;
3550+
std::map<std::string,
3551+
std::vector<std::pair<const Record *, FlattenedSpelling>>>
3552+
CXX, C23;
35553553

35563554
// Walk over the list of all attributes, and split them out based on the
35573555
// spelling variety.
@@ -3560,19 +3558,19 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
35603558
for (const auto &SI : Spellings) {
35613559
const std::string &Variety = SI.variety();
35623560
if (Variety == "GNU")
3563-
GNU.push_back(R);
3561+
GNU.emplace_back(R, SI);
35643562
else if (Variety == "Declspec")
3565-
Declspec.push_back(R);
3563+
Declspec.emplace_back(R, SI);
35663564
else if (Variety == "Microsoft")
3567-
Microsoft.push_back(R);
3565+
Microsoft.emplace_back(R, SI);
35683566
else if (Variety == "CXX11")
3569-
CXX[SI.nameSpace()].push_back(R);
3567+
CXX[SI.nameSpace()].emplace_back(R, SI);
35703568
else if (Variety == "C23")
3571-
C23[SI.nameSpace()].push_back(R);
3569+
C23[SI.nameSpace()].emplace_back(R, SI);
35723570
else if (Variety == "Pragma")
3573-
Pragma.push_back(R);
3571+
Pragma.emplace_back(R, SI);
35743572
else if (Variety == "HLSLSemantic")
3575-
HLSLSemantic.push_back(R);
3573+
HLSLSemantic.emplace_back(R, SI);
35763574
}
35773575
}
35783576

@@ -3594,7 +3592,10 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
35943592
OS << " return llvm::StringSwitch<int>(Name)\n";
35953593
GenerateHasAttrSpellingStringSwitch(HLSLSemantic, OS, "HLSLSemantic");
35963594
auto fn = [&OS](const char *Spelling,
3597-
const std::map<std::string, std::vector<Record *>> &List) {
3595+
const std::map<
3596+
std::string,
3597+
std::vector<std::pair<const Record *, FlattenedSpelling>>>
3598+
&List) {
35983599
OS << "case AttributeCommonInfo::Syntax::AS_" << Spelling << ": {\n";
35993600
// C++11-style attributes are further split out based on the Scope.
36003601
for (auto I = List.cbegin(), E = List.cend(); I != E; ++I) {

0 commit comments

Comments
 (0)