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

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.


================
Comment at: clang/lib/Headers/stdnoreturn.h:19
+/* The noreturn macro is deprecated in C2x. */
+#pragma clang deprecated(noreturn)
+
----------------
Is this actually useful? Isn't it sufficient to have the include-time warning for this header?


================
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
----------------
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'. "


================
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}}
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119707