aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.
Thank you for the patch!
I don't think this is an improvement; the diagnostic text goes from being
correct in C but odd in C++ to being correct in C++ but odd in C (the two
standards use different terminology). We're already inconsistent with which we
prefer (we have diagnostics that use `literal` and others that use `constant`),
and perhaps it'd make sense to start using `%select{constant|literal}N` in the
individual diagnostics to switch the wording based on the current language
mode. That said, I don't know that there's a ton of value in that change (it
seems unlikely that anyone is actually confused what the diagnostic is saying
today).
Also, this changes the text of a diagnostic but doesn't update any tests, so
precommit CI is failing. :-)
Before changing things in this review, I think we should go back to the bug
report to find out why the request was made and what issues there are with the
diagnostic. If it's a matter of "wrong terminology in a language mode", I'm of
the opinion that's not really an issue in this particular case (unless there's
other confusion that's caused by the terminology). I'll start the discussion
there.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119405/new/
https://reviews.llvm.org/D119405
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
