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

Reply via email to