aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6701-6707
if (!AL.isStmtAttr()) {
// Type attributes are handled elsewhere; silently move on.
assert(AL.isTypeAttr() && "Non-type attribute not handled");
break;
}
S.Diag(AL.getLoc(), diag::err_stmt_attribute_invalid_on_decl)
<< AL << D->getLocation();
----------------
If we're in the situation where we parse a plugin-based attribute (so
`AL.getKind()` falls into the `default` label) and its `handleDeclAttributes()`
(incorrectly) returns false rather than true, it seems like we would not want
to trigger either of these tests, correct? I'm thinking about a case where the
plugin knows about that attribute but cannot apply it because of some other
issues (maybe the subject is wrong, or args are incorrect, etc) and returns
`false` here. We wouldn't want to assert or claim that this is an invalid
statement attribute applied to a decl in that case.
Rather than requiring the user to return true from `handleDeclAttribute()`, it
seems that what we really want is something more like:
```
if (AL.getInfo().hasHandlerFunc()) {
AL.getInfo().handleDeclAttribute(S, D, AL);
break;
}
if (!AL.isStmtAttr()) {
...
}
S.Diag(...);
```
where an unknown attribute does not have a handler, but simple and plugin
attributes do have a handler. This way the user doesn't have the chance to
accidentally get confused about what it means to handle an attribute. WDYT?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D31342/new/
https://reviews.llvm.org/D31342
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
