[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D66364#1633775 <https://reviews.llvm.org/D66364#1633775>, @rsmith wrote:

> `_Thread_local` is a reserved identifier; we generally don't produce extension warnings for uses of reserved identifiers. (Eg, there's no warning for `_Atomic` in C++ or `_Bool` in C89, and no warning for uses of `__type_traits` or `__builtins`.)
>
> But I note that we *do* warn for some of these already (eg, `_Generic`, `_Static_assert`, `_Alignas`, and `_Alignof` get a warning). We should choose a rule and apply it consistently.
>
> What's the motivation for warning on this? Maybe that can inform whether these warnings are useful in general.


My motivation is for portability. _Thread_local (and all the rest) do not exist in C99 or earlier (or C++), so having some way to warn users of that is useful. I agree that we should be consistent and go with all or none, but my preference is for all (esp since this is a -pedantic warning class).

That said, I think `__builtins` and `__type_traits` are a separate question, and a somewhat interesting one. I could see some value in a pedantic warning for use of a reserved identifier which is an implementation detail outside of a system header in theory, but I'm not entirely sure what such a diagnostic would look like in practice. Is `__clang__` an implementation detail? I don't think we should warn on uses of it! What about `_UNICODE` when working with MSVC compat? Same situation. Rather than try to answer that question, I think I draw the line with only pedantically warning on reserved identifiers in the standard. WDYT?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66364