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

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 23 15:24:48 PDT 2019


rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

In D66364#1638026 <https://reviews.llvm.org/D66364#1638026>, @aaron.ballman wrote:

> @rsmith are you fine with implementing the diagnostic for these keywords piecemeal based on the pattern from this patch, or do you want to see an omnibus patch that adds all of them at once?


I'm fine with doing it piecemeal.



================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:130
+def ext_c11_feature : Extension<
   "%0 is a C11-specific feature">, InGroup<C11>;
 
----------------
Please consider rewording this to "%0 is a C11 extension" to match what we do for C++XY extensions. (This will also start looking a little silly once C20 adoption starts; features aren't C11-specific if they're also part of C20.)

As a separate change, though :)


================
Comment at: clang/test/Sema/thread-specifier.c:127-160
+// expected-warning at 14 {{_Thread_local is a C11-specific feature}}
+// expected-warning at 15 {{_Thread_local is a C11-specific feature}}
+// expected-warning at 16 {{_Thread_local is a C11-specific feature}}
+// expected-warning at 22 {{_Thread_local is a C11-specific feature}}
+// expected-warning at 23 {{_Thread_local is a C11-specific feature}}
+// expected-warning at 31 {{_Thread_local is a C11-specific feature}}
+// expected-warning at 40 {{_Thread_local is a C11-specific feature}}
----------------
Hardcoding line numbers like this makes test maintenance painful. Using a different verify prefix seems like the easiest way to handle this:

```
// RUN: %clang_cc1 [...] -D__thread=_Thread_local -std=c++98 -verify=expected,thread-local
[...]
__thread int t1; // thread-local-warning {{_Thread_local is a C11-specific feature}}
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D66364





More information about the cfe-commits mailing list