aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11087
+// TCB warnings
+def err_tcb_conflicting_attributes : Error<
+ "attributes '%0(\"%2\")' and '%1(\"%2\")' are mutually exclusive">;
----------------
NoQ wrote:
> aaron.ballman wrote:
> > NoQ wrote:
> > > Do i understand correctly that while "unknown attribute" is a warning
> > > ("must be an attribute for some other compiler, let's ignore"), misuse of
> > > a known attribute is typically an error ("ok, whatever you meant here, i
> > > have an opinion about what this really means and i don't like it")?
> > It depends strongly on the attribute and the problem at hand. My usual rule
> > of thumb is: if the attribute is being ignored because the code makes no
> > sense, it's an error, but if the attribute can still be useful (perhaps
> > after some adjustment), then warn. e.g., putting a calling convention
> > attribute on a variable of type `int` makes no sense -- that should
> > probably err rather than warn and ignore because the user did something
> > baffling.
> >
> > I could go either way on this one. Giving an error and forcing the user to
> > say what they mean is appealing, but it would also be defensible to warn
> > about the conflicting attribute and ignore just that one (retaining the
> > original attribute). I would say leave it as an error and we can downgrade
> > to a warning later if there's feedback from compelling real world use cases.
> > retaining the original attribute
>
> There's an interesting aspect of error recovery here (which i didn't address
> yet but added FIXMEs). We probably shouldn't emit additional warnings based
> on the attributes about which we've already reported a conflict error; that's
> just unnecessary clutter. That'd mean that we should always discard the
> non-leaf attribute and keep the leaf attribute (if we can at all discard
> attributes). Or keep both and repeat the conflict check during checking in
> order to suppress the warning.
> That'd mean that we should always discard the non-leaf attribute and keep the
> leaf attribute (if we can at all discard attributes). Or keep both and repeat
> the conflict check during checking in order to suppress the warning.
You can do `TheDecl->dropAttr<EnforceTCB>();` to drop the non-leaf attributes
(or, if easy, it's better to not add the problematic attribute in the first
place). Would you like to do that work in this patch or as a follow-up? (I'm
fine either way.)
================
Comment at: clang/lib/Sema/SemaChecking.cpp:16088
+ // is in a TCB that the Callee is not.
+ for (const auto *Attr : Caller->specific_attrs<EnforceTCBAttr>()) {
+ StringRef CallerTCB = Attr->getTCBName();
----------------
Can you rename this to be something other than a type name?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91898/new/
https://reviews.llvm.org/D91898
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
