[PATCH] D119707: [C2x] Implement WG14 N2764 the [[noreturn]] attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Headers/stdnoreturn.h:19
+/* The noreturn macro is deprecated in C2x. */
+#pragma clang deprecated(noreturn)
+
----------------
jyknight wrote:
> Is this actually useful? Isn't it sufficient to have the include-time warning for this header?
I think it gives a more precise diagnostic than the header inclusion one, and it will diagnose closer to where the use happens instead of at the include. Might not be the most useful, but it I think there's some utility.


================
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
----------------
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>`


================
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 at stdnoreturn.h:* {{macro marked 'deprecated' here}}
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119707