nickdesaulniers marked an inline comment as done.
nickdesaulniers added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1205
def BackendOptimizationFailure : DiagGroup<"pass-failed">;
+def BackendUserDiagnostic : DiagGroup<"user-diagnostic">;
----------------
aaron.ballman wrote:
> nickdesaulniers wrote:
> > aaron.ballman wrote:
> > > nickdesaulniers wrote:
> > > > aaron.ballman wrote:
> > > > > nickdesaulniers wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > GCC doesn't seem to support this spelling -- do they have a
> > > > > > > different one we should reuse?
> > > > > > I think these diagnostics don't have corresponding flags in GCC, so
> > > > > > they cannot be disabled.
> > > > > >
> > > > > > Without adding this, clang/test/Misc/warning-flags.c would fail,
> > > > > > because I was adding a new unnamed warning
> > > > > > `warn_fe_backend_user_diagnostic`. Perhaps I should not be adding
> > > > > > this line here, and doing something else?
> > > > > >
> > > > > > That test says we shouldn't be adding new warnings not controlled
> > > > > > by flags, but that's very much my intention.
> > > > > >
> > > > > > Though now `-Wno-user-diagnostic` doesn't produce a
> > > > > > `-Wunknown-warning-option` diagnostic...
> > > > > Ah! I think the warning attribute should be controllable via a
> > > > > diagnostic flag (we should always be able to disable warnings with
> > > > > some sort of flag) and I don't think the error attribute should be
> > > > > controllable (an error is an error and should stop translation, same
> > > > > as with `#error`).
> > > > >
> > > > > Normally, I'd say "let's use the same flag that controls `#warning`"
> > > > > but...
> > > > > ```
> > > > > def PoundWarning : DiagGroup<"#warnings">;
> > > > > ```
> > > > > That's... not exactly an obvious flag for the warning attribute. So I
> > > > > would probably name it `warning-attributes` similar to how we have
> > > > > `deprecated-attributes` already.
> > > > Done, now `-Wno-warning-attributes` doesn't produce
> > > > `-Wunknown-warning-option`, but also doesn't disable the diagnostic.
> > > > Was there something else I needed to add?
> > > huh, that sounds suspicious -- you don't need to do anything special for
> > > `-Wno-foo` handling, that should be automatically supported via tablegen.
> > > I'm not certain why you're not seeing `-Wno-warning-attributes` silencing
> > > the warnings.
> > Ah! Because I was testing `__attribute__((error("")))` not
> > `__attribute__((warning("")))`. `-Wno-warning-attributes` only affects the
> > latter, not the former. I should add a test for `-Wno-warning-attributes`!
> > Is this something I also need to add documentation for?
> Given that this behavior surprised you, I certainly wouldn't oppose
> mentioning it in the documentation, but I also don't think it's strictly
> required. That said, I do agree about adding some additional test coverage
> for when the warning is disabled.
>
> Just to double-check: do you think this functionality needs an "escape hatch"
> for the error case? e.g., should the `error` attribute generate a warning
> diagnostic that defaults to being an error, so we have
> `-Wno-warning-attributes` to turn off `warning` attribute diagnostics and
> `-Wno-error-attributes` to turn off `error` attribute diagnostics?
Ah, GCC also controls this via `-Wattribute-warning`; let me rename my
implementation.
> do you think this functionality needs an "escape hatch" for the error case?
No. If users don't want an error, then they should prefer
`__attribute__((warning("")))` to `__attribute__((error("")))`.
================
Comment at: llvm/include/llvm/IR/DiagnosticInfo.h:1079
+public:
+ // TODO: make LocCookie optional
+ DiagnosticInfoDontCall(StringRef CalleeName, unsigned LocCookie)
----------------
nickdesaulniers wrote:
> TODO: play with llvm::Optional
Ok, that was fun, but I didn't find it made construction sites of
`DiagnosticInfoDontCall` nicer. This is also how `DiagnosticInfoInlineAsm`
works, which is where I got the idea for `!srcloc` from.
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
