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@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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to