[PATCH] Emit an extension warning when changing system header tokens

Alp Toker alp at nuanti.com
Thu Nov 28 04:27:51 PST 2013


clang converts keywords to identifiers for compatibility with various 
system headers such as GNU libc.

The attached patch adds an extension warning diagnostic in those cases. 
The warning is typically ignored by default when the issue is 
encountered in system headers, but can be enabled to aid in standards 
conformance testing.

This also changes the __uptr keyword avoidance from r195710 to not be 
special-cased for system headers now that a warning is in place, 
bringing it in line with other similar workarounds in clang.

Some examples:

|warning: keyword '__is_pod' will be treated as an identifier for the 
remainder of the translation unit [-Wkeyword-compat]||
||struct __is_pod||
||
||warning: keyword '__uptr' will be treated as an identifier here 
[-Wkeyword-compat]||
||union w *__uptr;|

Alp.

-- 
http://www.nuanti.com
the browser experts

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131128/798b453e/attachment.html>
-------------- next part --------------
diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index 6de1a48..ec1eb1a 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -50,6 +50,7 @@ def BuiltinRequiresHeader : DiagGroup<"builtin-requires-header">;
 def C99Compat : DiagGroup<"c99-compat">;
 def CXXCompat: DiagGroup<"c++-compat">;
 def ExternCCompat : DiagGroup<"extern-c-compat">;
+def KeywordCompat : DiagGroup<"keyword-compat">;
 def GNUCaseRange : DiagGroup<"gnu-case-range">;
 def CastAlign : DiagGroup<"cast-align">;
 def : DiagGroup<"cast-qual">;
diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td
index 4d0641d..2feab52 100644
--- a/include/clang/Basic/DiagnosticParseKinds.td
+++ b/include/clang/Basic/DiagnosticParseKinds.td
@@ -58,6 +58,9 @@ def ext_plain_complex : ExtWarn<
 def ext_integer_complex : Extension<
   "complex integer types are a GNU extension">, InGroup<GNUComplexInteger>;
 def ext_thread_before : Extension<"'__thread' before '%0'">;
+def ext_keyword_as_ident : ExtWarn<
+  "keyword '%0' will be treated as an identifier %select{here|for the remainder of the translation unit}1">,
+  InGroup<KeywordCompat>;
 
 def error_empty_enum : Error<"use of empty enum">;
 def err_invalid_sign_spec : Error<"'%0' cannot be signed or unsigned">;
diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index 8807e3b..302bfed 100644
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -563,6 +563,13 @@ private:
                                 const char *&PrevSpec, unsigned &DiagID,
                                 bool &isInvalid);
 
+  /// TryIdentifierTokenFallback - For compatibility with system headers using
+  /// keywords as identifiers, attempt to convert the current token to an
+  /// identifier and optionally disable the keyword for the remainder of the
+  /// translation unit. This returns false if the token was not replaced,
+  /// otherwise emits a diagnostic and returns true.
+  bool TryIdentifierTokenFallback(bool DisableKeyword);
+
   /// \brief Get the TemplateIdAnnotation from the token.
   TemplateIdAnnotation *takeTemplateIdAnnotation(const Token &tok);
 
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 881e225..2cc6ecf 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -2739,10 +2739,8 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
       // then treat __is_signed as an identifier rather than as a keyword.
       if (DS.getTypeSpecType() == TST_bool &&
           DS.getTypeQualifiers() == DeclSpec::TQ_const &&
-          DS.getStorageClassSpec() == DeclSpec::SCS_static) {
-        Tok.getIdentifierInfo()->RevertTokenIDToIdentifier();
-        Tok.setKind(tok::identifier);
-      }
+          DS.getStorageClassSpec() == DeclSpec::SCS_static)
+        TryIdentifierTokenFallback(true);
 
       // We're done with the declaration-specifiers.
       goto DoneWithDeclSpec;
@@ -4488,10 +4486,9 @@ void Parser::ParseTypeQualifierListOpt(DeclSpec &DS,
       // GNU libc headers in C mode use '__uptr' as an identifer which conflicts
       // with the MS modifier keyword.
       if (VendorAttributesAllowed && !getLangOpts().CPlusPlus &&
-          IdentifierRequired && DS.isEmpty() && NextToken().is(tok::semi) &&
-          PP.getSourceManager().isInSystemHeader(Loc)) {
-        Tok.setKind(tok::identifier);
-        continue;
+          IdentifierRequired && DS.isEmpty() && NextToken().is(tok::semi)) {
+        if (TryIdentifierTokenFallback(false))
+          continue;
       }
     case tok::kw___sptr:
     case tok::kw___w64:
diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp
index 32e151c..103bbf7 100644
--- a/lib/Parse/ParseDeclCXX.cpp
+++ b/lib/Parse/ParseDeclCXX.cpp
@@ -1198,15 +1198,13 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
        Tok.is(tok::kw___is_scalar) ||
        Tok.is(tok::kw___is_signed) ||
        Tok.is(tok::kw___is_unsigned) ||
-       Tok.is(tok::kw___is_void))) {
+       Tok.is(tok::kw___is_void)))
     // GNU libstdc++ 4.2 and libc++ use certain intrinsic names as the
     // name of struct templates, but some are keywords in GCC >= 4.3
     // and Clang. Therefore, when we see the token sequence "struct
     // X", make X into a normal identifier rather than a keyword, to
     // allow libstdc++ 4.2 and libc++ to work properly.
-    Tok.getIdentifierInfo()->RevertTokenIDToIdentifier();
-    Tok.setKind(tok::identifier);
-  }
+    TryIdentifierTokenFallback(true);
 
   // Parse the (optional) nested-name-specifier.
   CXXScopeSpec &SS = DS.getTypeSpecScope();
diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp
index 0f950ce..5559593 100644
--- a/lib/Parse/Parser.cpp
+++ b/lib/Parse/Parser.cpp
@@ -1517,6 +1517,17 @@ Parser::TryAnnotateName(bool IsAddressOfOperand,
   return ANK_Unresolved;
 }
 
+bool Parser::TryIdentifierTokenFallback(bool DisableKeyword) {
+  assert(Tok.isNot(tok::identifier));
+  Diag(Tok, diag::ext_keyword_as_ident)
+    << PP.getSpelling(Tok)
+    << DisableKeyword;
+  if (DisableKeyword)
+    Tok.getIdentifierInfo()->RevertTokenIDToIdentifier();
+  Tok.setKind(tok::identifier);
+  return true;
+}
+
 /// TryAnnotateTypeOrScopeToken - If the current token position is on a
 /// typename (possibly qualified in C++) or a C++ scope specifier not followed
 /// by a typename, TryAnnotateTypeOrScopeToken will replace one or more tokens
diff --git a/test/PCH/cxx-traits.cpp b/test/PCH/cxx-traits.cpp
index 938f36f..ffdfccc 100644
--- a/test/PCH/cxx-traits.cpp
+++ b/test/PCH/cxx-traits.cpp
@@ -2,9 +2,11 @@
 // RUN: %clang_cc1 -include %S/cxx-traits.h -std=c++11 -fsyntax-only -verify %s
 
 // RUN: %clang_cc1 -x c++-header -std=c++11 -emit-pch -o %t %S/cxx-traits.h
-// RUN: %clang_cc1 -std=c++11 -include-pch %t -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -include-pch %t -DPCH -fsyntax-only -verify %s
 
+#ifdef PCH
 // expected-no-diagnostics
+#endif
 
 bool _Is_pod_comparator = __is_pod<int>::__value;
 bool _Is_empty_check = __is_empty<int>::__value;
diff --git a/test/PCH/cxx-traits.h b/test/PCH/cxx-traits.h
index 8b62002..836804e 100644
--- a/test/PCH/cxx-traits.h
+++ b/test/PCH/cxx-traits.h
@@ -1,12 +1,12 @@
 // Header for PCH test cxx-traits.cpp
 
 template<typename _Tp>
-struct __is_pod {
+struct __is_pod { // expected-warning {{keyword '__is_pod' will be treated as an identifier for the remainder of the translation unit}}
   enum { __value };
 };
 
 template<typename _Tp>
-struct __is_empty {
+struct __is_empty { // expected-warning {{keyword '__is_empty' will be treated as an identifier for the remainder of the translation unit}}
   enum { __value };
 };
 
diff --git a/test/Sema/Inputs/ms-keyword-system-header.h b/test/Sema/Inputs/ms-keyword-system-header.h
index 13cfe3a..43a3db7 100644
--- a/test/Sema/Inputs/ms-keyword-system-header.h
+++ b/test/Sema/Inputs/ms-keyword-system-header.h
@@ -2,5 +2,8 @@
 
 typedef union {
   union w *__uptr;
+#if defined(MS) && defined(NOT_SYSTEM)
+  // expected-warning at -2 {{keyword '__uptr' will be treated as an identifier here}}
+#endif
   int *__iptr;
 } WS __attribute__((__transparent_union__));
diff --git a/test/Sema/ms-keyword-system-header.c b/test/Sema/ms-keyword-system-header.c
index bf7a9f4..b4ff568 100644
--- a/test/Sema/ms-keyword-system-header.c
+++ b/test/Sema/ms-keyword-system-header.c
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -fms-extensions -D MS -isystem %S/Inputs %s -fsyntax-only -verify
+// RUN: %clang_cc1 -fms-extensions -D MS -Wno-keyword-compat -I %S/Inputs %s -fsyntax-only -verify
+// RUN: %clang_cc1 -fms-extensions -D MS -D NOT_SYSTEM -I %S/Inputs %s -fsyntax-only -verify
 // RUN: %clang_cc1 -isystem %S/Inputs %s -fsyntax-only -verify
 
 // PR17824: GNU libc uses MS keyword __uptr as an identifier in C mode


More information about the cfe-commits mailing list