nickdesaulniers added inline comments.
================
Comment at: clang/test/Sema/attr-error.c:31-32
+
+__attribute__((error("foo"))) int bad5(void); // expected-error {{'error'
and 'warning' attributes are not compatible}}
+__attribute__((warning("foo"))) int bad5(void); // expected-note {{conflicting
attribute is here}}
+
----------------
aaron.ballman wrote:
> I think the diagnostic order is backwards here. The first declaration is
> where I'd expect the note and the second declaration is where I'd expect the
> error. (The idea is: the first declaration adds an attribute to the decl, so
> the redeclaration is what introduces the conflict and so that's where the
> error should live.) As an example: https://godbolt.org/z/bjGTWxYvh
I'm not sure how to reconcile this. Looking at `bad4` above (different case
than `bad5`), the diagnostic we get is:
```
/tmp/y.c:1:30: error: 'warning' and 'error' attributes are not compatible
__attribute__((error("foo"), warning("foo")))
^
/tmp/y.c:1:16: note: conflicting attribute is here
__attribute__((error("foo"), warning("foo")))
^
1 error generated.
```
which looks correct to me. If I flip the order these are diagnosed in, that
fixes `bad5` but if I flip around the ordering:
```
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
- Diag(CI.getLoc(), diag::err_attributes_are_not_compatible) << CI << EA;
- Diag(EA->getLocation(), diag::note_conflicting_attribute);
+ Diag(EA->getLocation(), diag::err_attributes_are_not_compatible)
+ << CI << EA;
+ Diag(CI.getLoc(), diag::note_conflicting_attribute);
```
Then `bad4` doesn't look quite correct.
```
/tmp/y.c:1:16: error: 'warning' and 'error' attributes are not compatible
__attribute__((error("foo"), warning("foo")))
^
/tmp/y.c:1:30: note: conflicting attribute is here
__attribute__((error("foo"), warning("foo")))
^
```
Perhaps in the callers of `handleErrorAttr` I'm confusing which `Decl` is the
"new" vs the "old?"
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106030/new/
https://reviews.llvm.org/D106030
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
