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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits