[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This is right on the line of what I think is reasonable in terms of diagnostics from the backend. It provides useful information to users about inlining decisions, so that's awesome when that information is helpful. But those inlining decisions can be arbitrarily long and complex, so this can emit a *lot* of note diagnostics on real world code which can make the diagnostic harder to reason about. In general, I think inlining decisions should be left to an optimization report, but this information could make interpreting the `error` or `warning` attributes easier.

Have you checked to see how the diagnostics work in practice on deep inlining stacks? If it's usually only 1-2 extra notes, that's probably not going to overwhelm anyone. But if it's significantly more, that could be distracting. Do we want any options to control when to emit the notes? e.g., do we want to give users a way to say "only emit the last N inlining decision notes"?

Also, how does this work with LTO when it makes different inlining decisions?



================
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())
----------------
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.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141451/new/

https://reviews.llvm.org/D141451