jyknight added inline comments.
================
Comment at: clang/lib/Headers/stdnoreturn.h:22
+/* Including the header file in C2x is also deprecated. */
+#warning "the '<stdnoreturn.h>' header is deprecated in C2x"
+#endif
----------------
aaron.ballman wrote:
> jyknight wrote:
> > Would be nice to mention the solution, as well. E.g.
> >
> > "The '<stdnoreturn.h>' header has been deprecated in C2x: including it no
> > longer necessary in order to use 'noreturn'. "
> I can add that, but it is necessary in order to use 'noreturn' as a function
> specifier, so that wording isn't quite right. e.g., `noreturn void
> func(void);` is valid C today if you `#include <stdnoreturn.h>`
Ah, I missed that subtlety. Maybe say:
"The '<stdnoreturn.h>' header has been deprecated in C2x; either use the
_Noreturn keyword or the [[noreturn]] attribute."
================
Comment at: clang/test/Sema/c2x-noreturn.c:41-43
+[[noreturn]] void func6(void); // all-warning {{the '[[_Noreturn]]' attribute
spelling is deprecated in C2x; use '[[noreturn]]' instead}} \
+ // c2x-warning {{macro 'noreturn' has been
marked as deprecated}} \
+ // c2x-note@stdnoreturn.h:* {{macro marked
'deprecated' here}}
----------------
aaron.ballman wrote:
> jyknight wrote:
> > If you've written [[noreturn]] in your code, you're doing the right thing
> > already. Why bother emitting a warning? The problem that should be
> > corrected is at the point of inclusion of stdnoreturn.h, not the spelling
> > here.
> >
> > I'd suggest only warning about "[[_Noreturn]]" if the user actually
> > //wrote// it like that, and not when it's expanded from the "noreturn"
> > macro.
> > I'd suggest only warning about "[[_Noreturn]]" if the user actually wrote
> > it like that, and not when it's expanded from the "noreturn" macro.
>
> I'm not keen on that design. The goal of this diagnostic is to let people
> know that they have a surprise in their code (that this macro is expanding to
> something they may not expect) at the location the surprise happens. Consider:
>
> ```
> // TU.c
> #include <stdnoreturn.h>
> #include "my_header.h"
>
> // my_header.h
> [[noreturn]] void func(void);
> ```
> my_header.h doesn't see the inclusion of stdnoreturn.h, so finding out about
> the macro may inform the developer to be more protective of using the
> attribute.
But that's exactly my point. There _is_ no surprise in this code. It works
exactly as expected.
The my_header.h code is using the new non-deprecated spelling, and is getting
the correct and desired behavior -- even though someone else has included
stdnoreturn.h. That it's working properly due to there existing a compatibility
attribute "_Noreturn" is basically an irrelevant detail to any user.
Emitting any warnings for this line of code seems potentially even harmful.
Consider: what should my_header.h be doing differently due to the warning?
Nothing. Ideally, they should change nothing. Yet, if we do emit warnings for
this usage, it'll likely cause some people to try to add push/undef/pop gunk to
their headers to avoid the warning, which makes the code worse than if they did
nothing.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119707/new/
https://reviews.llvm.org/D119707
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
