maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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.
This commit is contained in:
Aaron Ballman 2021-04-13 15:18:13 -04:00
parent 6aa6f748ae
commit c058a71227
6 changed files with 96 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
clang/include/clang/Sema/ParsedAttr.h
View File

@ -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 @@ public:
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>>

4
clang/lib/Sema/ParsedAttr.cpp
View File

@ -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);

4
clang/lib/Sema/SemaDecl.cpp
View File

@ -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,

13
clang/lib/Sema/SemaStmtAttr.cpp
View File

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

6
clang/test/SemaCXX/attr-likelihood.cpp
View File

@ -147,5 +147,11 @@ void o()
if constexpr (true) [[likely]] {
// expected-warning@+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

109
clang/utils/TableGen/ClangAttrEmitter.cpp
View File

@ -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 different 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