cjdb added inline comments.
================ Comment at: clang/test/Frontend/backend-attribute-error-warning-optimize.c:12 foo(); // expected-error {{call to 'foo' declared with 'error' attribute: oh no foo}} + // expected-note@* {{In function 'baz'}} if (x()) ---------------- aaron.ballman wrote: > nickdesaulniers wrote: > > nickdesaulniers wrote: > > > aaron.ballman wrote: > > > > Instead of allowing this note to appear anywhere in the file, I think > > > > it's better to use "bookmark" comments. e.g., > > > > ``` > > > > void baz() { // #baz_defn > > > > } > > > > > > > > void foo() { > > > > baz(); // expected-note@#baz_defn {{whatever note text}} > > > > } > > > > ``` > > > > so that we're testing where the diagnostic is emitted. (This is > > > > especially important given that the changes are about location > > > > tracking.) > > > oh, that's new (to me)! will do > > It looks like the "bookmarks" don't work because the notes do not line+col > > info. They follow the warning/error diagnostic which does, on the bottom > > most call site. > > > > The warning is supposed to be emitted on the callsite, not the definition. > I still don't think this is reasonable for test coverage because this means > we'll accept the note *anywhere* in the file. This makes it much too easy to > regress the behavior accidentally (e.g., accidentally emit all the notes on > line 1 of the file). I think we need *some* way to nail down where these > notes are emitted in the test. However, I might be asking you to solve > "please track location information through the backend" for that, so perhaps > this isn't a reasonable request? > > Also, CC @cjdb for awareness of how this kind of diagnostic is produced > (Chris is working on an idea for how we emit diagnostics so we get better > structured information from them, so knowing about this novel approach might > impact that idea). Very interesting, thanks for the heads up! Cross-phase diagnostics are definitely something I hadn't considered, and it **does** impact the design (but not in a derailing way). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141451/new/ https://reviews.llvm.org/D141451 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits