[clang] c058a71 - Correct the tablegen for checking mutually exclusive stmt attrs
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 13 12:20:39 PDT 2021
Author: Aaron Ballman
Date: 2021-04-13T15:20:30-04:00
New Revision: c058a7122787c8e503cf6306b8da03c0e56a41a5
URL: https://github.com/llvm/llvm-project/commit/c058a7122787c8e503cf6306b8da03c0e56a41a5
DIFF: https://github.com/llvm/llvm-project/commit/c058a7122787c8e503cf6306b8da03c0e56a41a5.diff
LOG: Correct the tablegen for checking mutually exclusive stmt attrs
The previous implementation was insufficient for checking statement
attribute mutual exclusion because attributed statements do not collect
their attributes one-at-a-time in the same way that declarations do. So
the design that was attempting to check for mutual exclusion as each
attribute was processed would not ever catch a mutual exclusion in a
statement. This was missed due to insufficient test coverage, which has
now been added for the [[likely]] and [[unlikely]] attributes.
The new approach is to check all of attributes that are to be applied
to the attributed statement in a group. This required generating
another DiagnoseMutualExclusions() function into AttrParsedAttrImpl.inc.
Added:
Modified:
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Sema/ParsedAttr.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaStmtAttr.cpp
clang/test/SemaCXX/attr-likelihood.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h
index 471a1ecfac9b2..e508b4651be1c 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -92,12 +92,6 @@ struct ParsedAttrInfo {
const Decl *D) const {
return true;
}
- /// Check if the given attribute is mutually exclusive with other attributes
- /// already applied to the given statement.
- virtual bool diagMutualExclusion(Sema &S, const ParsedAttr &A,
- const Stmt *St) const {
- return true;
- }
/// Check if this attribute is allowed by the language we are compiling, and
/// issue a diagnostic if not.
virtual bool diagLangOpts(Sema &S, const ParsedAttr &Attr) const {
@@ -612,7 +606,12 @@ class ParsedAttr final
bool diagnoseAppertainsTo(class Sema &S, const Decl *D) const;
bool diagnoseAppertainsTo(class Sema &S, const Stmt *St) const;
bool diagnoseMutualExclusion(class Sema &S, const Decl *D) const;
- bool diagnoseMutualExclusion(class Sema &S, const Stmt *St) const;
+ // This function stub exists for parity with the declaration checking code so
+ // that checkCommonAttributeFeatures() can work generically on declarations
+ // or statements.
+ bool diagnoseMutualExclusion(class Sema &S, const Stmt *St) const {
+ return true;
+ }
bool appliesToDecl(const Decl *D, attr::SubjectMatchRule MatchRule) const;
void getMatchRules(const LangOptions &LangOpts,
SmallVectorImpl<std::pair<attr::SubjectMatchRule, bool>>
diff --git a/clang/lib/Sema/ParsedAttr.cpp b/clang/lib/Sema/ParsedAttr.cpp
index 989db3c2a4064..ed03b0c7f688b 100644
--- a/clang/lib/Sema/ParsedAttr.cpp
+++ b/clang/lib/Sema/ParsedAttr.cpp
@@ -167,10 +167,6 @@ bool ParsedAttr::diagnoseMutualExclusion(Sema &S, const Decl *D) const {
return getInfo().diagMutualExclusion(S, *this, D);
}
-bool ParsedAttr::diagnoseMutualExclusion(Sema &S, const Stmt *St) const {
- return getInfo().diagMutualExclusion(S, *this, St);
-}
-
bool ParsedAttr::appliesToDecl(const Decl *D,
attr::SubjectMatchRule MatchRule) const {
return checkAttributeMatchRuleAppliesTo(D, MatchRule);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index ea9f0949eac25..1d57d8e848497 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2540,9 +2540,9 @@ static bool mergeAlignedAttrs(Sema &S, NamedDecl *New, Decl *Old) {
return AnyAdded;
}
-#define WANT_MERGE_LOGIC
+#define WANT_DECL_MERGE_LOGIC
#include "clang/Sema/AttrParsedAttrImpl.inc"
-#undef WANT_MERGE_LOGIC
+#undef WANT_DECL_MERGE_LOGIC
static bool mergeDeclAttribute(Sema &S, NamedDecl *D,
const InheritableAttr *Attr,
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index f914f1273f719..763c6b61fbd90 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -227,9 +227,22 @@ static Attr *handleUnlikely(Sema &S, Stmt *St, const ParsedAttr &A,
return ::new (S.Context) UnlikelyAttr(S.Context, A);
}
+#define WANT_STMT_MERGE_LOGIC
+#include "clang/Sema/AttrParsedAttrImpl.inc"
+#undef WANT_STMT_MERGE_LOGIC
+
static void
CheckForIncompatibleAttributes(Sema &S,
const SmallVectorImpl<const Attr *> &Attrs) {
+ // The vast majority of attributed statements will only have one attribute
+ // on them, so skip all of the checking in the common case.
+ if (Attrs.size() < 2)
+ return;
+
+ // First, check for the easy cases that are table-generated for us.
+ if (!DiagnoseMutualExclusions(S, Attrs))
+ return;
+
// There are 6 categories of loop hints attributes: vectorize, interleave,
// unroll, unroll_and_jam, pipeline and distribute. Except for distribute they
// come in two variants: a state form and a numeric form. The state form
diff --git a/clang/test/SemaCXX/attr-likelihood.cpp b/clang/test/SemaCXX/attr-likelihood.cpp
index d2f95d17069ca..03ba301251d78 100644
--- a/clang/test/SemaCXX/attr-likelihood.cpp
+++ b/clang/test/SemaCXX/attr-likelihood.cpp
@@ -147,5 +147,11 @@ void o()
if constexpr (true) [[likely]] {
// expected-warning at +1 {{attribute 'likely' has no effect when annotating an 'if constexpr' statement}}
} else [[likely]];
+
+ if (1) [[likely, unlikely]] { // expected-error {{'unlikely' and 'likely' attributes are not compatible}} \
+ // expected-note {{conflicting attribute is here}}
+ } else [[unlikely]][[likely]] { // expected-error {{'likely' and 'unlikely' attributes are not compatible}} \
+ // expected-note {{conflicting attribute is here}}
+ }
}
#endif
diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp
index ececceb5d3fd4..d679d58aaef17 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3633,7 +3633,8 @@ static void GenerateAppertainsTo(const Record &Attr, raw_ostream &OS) {
static void GenerateMutualExclusionsChecks(const Record &Attr,
const RecordKeeper &Records,
raw_ostream &OS,
- raw_ostream &MergeOS) {
+ raw_ostream &MergeDeclOS,
+ raw_ostream &MergeStmtOS) {
// Find all of the definitions that inherit from MutualExclusions and include
// the given attribute in the list of exclusions to generate the
// diagMutualExclusion() check.
@@ -3702,43 +3703,59 @@ static void GenerateMutualExclusionsChecks(const Record &Attr,
// Sema &S, Decl *D, Attr *A and that returns a bool (false on diagnostic,
// true on success).
if (Attr.isSubClassOf("InheritableAttr")) {
- MergeOS << " if (const auto *Second = dyn_cast<"
- << (Attr.getName() + "Attr").str() << ">(A)) {\n";
+ MergeDeclOS << " if (const auto *Second = dyn_cast<"
+ << (Attr.getName() + "Attr").str() << ">(A)) {\n";
for (const std::string &A : DeclAttrs) {
- MergeOS << " if (const auto *First = D->getAttr<" << A << ">()) {\n";
- MergeOS << " S.Diag(First->getLocation(), "
- << "diag::err_attributes_are_not_compatible) << First << "
- << "Second;\n";
- MergeOS << " S.Diag(Second->getLocation(), "
- << "diag::note_conflicting_attribute);\n";
- MergeOS << " return false;\n";
- MergeOS << " }\n";
+ MergeDeclOS << " if (const auto *First = D->getAttr<" << A
+ << ">()) {\n";
+ MergeDeclOS << " S.Diag(First->getLocation(), "
+ << "diag::err_attributes_are_not_compatible) << First << "
+ << "Second;\n";
+ MergeDeclOS << " S.Diag(Second->getLocation(), "
+ << "diag::note_conflicting_attribute);\n";
+ MergeDeclOS << " return false;\n";
+ MergeDeclOS << " }\n";
}
- MergeOS << " return true;\n";
- MergeOS << " }\n";
+ MergeDeclOS << " return true;\n";
+ MergeDeclOS << " }\n";
}
}
+
+ // Statement attributes are a bit
diff erent from declarations. With
+ // declarations, each attribute is added to the declaration as it is
+ // processed, and so you can look on the Decl * itself to see if there is a
+ // conflicting attribute. Statement attributes are processed as a group
+ // because AttributedStmt needs to tail-allocate all of the attribute nodes
+ // at once. This means we cannot check whether the statement already contains
+ // an attribute to check for the conflict. Instead, we need to check whether
+ // the given list of semantic attributes contain any conflicts. It is assumed
+ // this code will be executed in the context of a function with parameters:
+ // Sema &S, const SmallVectorImpl<const Attr *> &C. The code will be within a
+ // loop which loops over the container C with a loop variable named A to
+ // represent the current attribute to check for conflicts.
+ //
+ // FIXME: it would be nice not to walk over the list of potential attributes
+ // to apply to the statement more than once, but statements typically don't
+ // have long lists of attributes on them, so re-walking the list should not
+ // be an expensive operation.
if (!StmtAttrs.empty()) {
- // Generate the ParsedAttrInfo subclass logic for statements.
- OS << " bool diagMutualExclusion(Sema &S, const ParsedAttr &AL, "
- << "const Stmt *St) const override {\n";
- OS << " if (const auto *AS = dyn_cast<AttributedStmt>(St)) {\n";
- OS << " const ArrayRef<const Attr *> &Attrs = AS->getAttrs();\n";
- for (const std::string &A : StmtAttrs) {
- OS << " auto Iter" << A << " = llvm::find_if(Attrs, [](const Attr "
- << "*A) { return isa<" << A << ">(A); });\n";
- OS << " if (Iter" << A << " != Attrs.end()) {\n";
- OS << " S.Diag(AL.getLoc(), "
- << "diag::err_attributes_are_not_compatible) << AL << *Iter" << A
- << ";\n";
- OS << " S.Diag((*Iter" << A << ")->getLocation(), "
- << "diag::note_conflicting_attribute);\n";
- OS << " return false;\n";
- OS << " }\n";
- }
- OS << " }\n";
- OS << " return true;\n";
- OS << " }\n\n";
+ MergeStmtOS << " if (const auto *Second = dyn_cast<"
+ << (Attr.getName() + "Attr").str() << ">(A)) {\n";
+ MergeStmtOS << " auto Iter = llvm::find_if(C, [](const Attr *Check) "
+ << "{ return isa<";
+ interleave(
+ StmtAttrs, [&](const std::string &Name) { MergeStmtOS << Name; },
+ [&] { MergeStmtOS << ", "; });
+ MergeStmtOS << ">(Check); });\n";
+ MergeStmtOS << " if (Iter != C.end()) {\n";
+ MergeStmtOS << " S.Diag((*Iter)->getLocation(), "
+ << "diag::err_attributes_are_not_compatible) << *Iter << "
+ << "Second;\n";
+ MergeStmtOS << " S.Diag(Second->getLocation(), "
+ << "diag::note_conflicting_attribute);\n";
+ MergeStmtOS << " return false;\n";
+ MergeStmtOS << " }\n";
+ MergeStmtOS << " }\n";
}
}
@@ -3890,7 +3907,8 @@ static bool IsKnownToGCC(const Record &Attr) {
void EmitClangAttrParsedAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
emitSourceFileHeader("Parsed attribute helpers", OS);
- OS << "#if !defined(WANT_MERGE_LOGIC)\n";
+ OS << "#if !defined(WANT_DECL_MERGE_LOGIC) && "
+ << "!defined(WANT_STMT_MERGE_LOGIC)\n";
PragmaClangAttributeSupport &PragmaAttributeSupport =
getPragmaAttributeSupport(Records);
@@ -3914,8 +3932,8 @@ void EmitClangAttrParsedAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
// This stream is used to collect all of the declaration attribute merging
// logic for performing mutual exclusion checks. This gets emitted at the
// end of the file in a helper function of its own.
- std::string DeclMergeChecks;
- raw_string_ostream MergeOS(DeclMergeChecks);
+ std::string DeclMergeChecks, StmtMergeChecks;
+ raw_string_ostream MergeDeclOS(DeclMergeChecks), MergeStmtOS(StmtMergeChecks);
// Generate a ParsedAttrInfo struct for each of the attributes.
for (auto I = Attrs.begin(), E = Attrs.end(); I != E; ++I) {
@@ -3970,7 +3988,7 @@ void EmitClangAttrParsedAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
OS << " Spellings = " << I->first << "Spellings;\n";
OS << " }\n";
GenerateAppertainsTo(Attr, OS);
- GenerateMutualExclusionsChecks(Attr, Records, OS, MergeOS);
+ GenerateMutualExclusionsChecks(Attr, Records, OS, MergeDeclOS, MergeStmtOS);
GenerateLangOptRequirements(Attr, OS);
GenerateTargetRequirements(Attr, Dupes, OS);
GenerateSpellingIndexToSemanticSpelling(Attr, OS);
@@ -3991,16 +4009,27 @@ void EmitClangAttrParsedAttrImpl(RecordKeeper &Records, raw_ostream &OS) {
// Generate the attribute match rules.
emitAttributeMatchRules(PragmaAttributeSupport, OS);
- OS << "#else // WANT_MERGE_LOGIC\n\n";
+ OS << "#elif defined(WANT_DECL_MERGE_LOGIC)\n\n";
// Write out the declaration merging check logic.
OS << "static bool DiagnoseMutualExclusions(Sema &S, const NamedDecl *D, "
<< "const Attr *A) {\n";
- OS << MergeOS.str();
+ OS << MergeDeclOS.str();
OS << " return true;\n";
OS << "}\n\n";
- OS << "#endif // WANT_MERGE_LOGIC\n";
+ OS << "#elif defined(WANT_STMT_MERGE_LOGIC)\n\n";
+
+ // Write out the statement merging check logic.
+ OS << "static bool DiagnoseMutualExclusions(Sema &S, "
+ << "const SmallVectorImpl<const Attr *> &C) {\n";
+ OS << " for (const Attr *A : C) {\n";
+ OS << MergeStmtOS.str();
+ OS << " }\n";
+ OS << " return true;\n";
+ OS << "}\n\n";
+
+ OS << "#endif\n";
}
// Emits the kind list of parsed attributes
More information about the cfe-commits
mailing list