[PATCH] D29868: Recover more gracefully when __declspec is not supported as a keyword

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.

In r238238, we removed __declspec as a universally-accepted keyword -- instead, it is only enabled as a supported keyword when -fms-extensions or -fdeclspec is passed to the driver. However, this had an unfortunate side-effect in that it made for bad diagnostics in the presence of a __declspec when neither of those flags are passed. For instance:

__declspec(naked) void f(void) {}

clang -fsyntax-only -fno-ms-extensions -Wall -pedantic main.c

main.cpp:1:24: error: parameter named 'f' is missing
__declspec(naked) void f(void) {}

  ^

main.cpp:1:31: error: expected ';' at end of declaration
__declspec(naked) void f(void) {}

  ^
  ;

main.cpp:1:12: warning: parameter 'naked' was not declared, defaulting to type 'int' [-Wpedantic]
__declspec(naked) void f(void) {}

  ^

main.cpp:1:1: warning: type specifier missing, defaults to 'int' [-Wimplicit-int]
__declspec(naked) void f(void) {}

This patch addresses the poor diagnostics by noticing when a __declspec has been used but is not enabled. It eats the attribute, diagnoses the use, and then attempts to continue parsing.


https://reviews.llvm.org/D29868

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  lib/Parse/ParseDecl.cpp
  test/Parser/declspec-recovery.c
  test/Parser/declspec-supported.c


Index: test/Parser/declspec-supported.c
===================================================================
--- test/Parser/declspec-supported.c
+++ test/Parser/declspec-supported.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -fms-extensions -verify %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -fdeclspec -verify %s
+// expected-no-diagnostics
+
+__declspec(naked) void f(void) {}
+
+struct S {
+  __declspec(property(get=Getter, put=Setter)) int X;
+  int Y;
+};
Index: test/Parser/declspec-recovery.c
===================================================================
--- test/Parser/declspec-recovery.c
+++ test/Parser/declspec-recovery.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -verify %s
+
+__declspec(naked) void f(void) {} // expected-error{{'__declspec' attributes are not enabled; use '-fdeclspec' or '-fms-extensions' to enable support for __declspec attributes}}
+
+struct S {
+  __declspec(property(get=Getter, put=Setter)) int X; // expected-error{{'__declspec' attributes are not enabled; use '-fdeclspec' or '-fms-extensions' to enable support for __declspec attributes}}
+  int Y;
+};
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -2966,6 +2966,31 @@
       if (DS.hasTypeSpecifier())
         goto DoneWithDeclSpec;
 
+      // If the token is an identifier named "__declspec" and Microsoft
+      // extensions are not enabled, it is likely that there will be cascading
+      // parse errors if this really is a __declspec attribute. Attempt to
+      // recognize that scenario and recover gracefully.
+      if (!getLangOpts().MicrosoftExt && Tok.is(tok::identifier) &&
+          Tok.getIdentifierInfo()->getName().equals("__declspec")) {
+        // The next token should be an open paren. If it is, eat the entire
+        // attribute declaration and continue.
+        if (NextToken().is(tok::l_paren)) {
+          // Consume the __declspec identifier.
+          SourceLocation Loc = ConsumeToken();
+
+          // Eat the parens and everything between them.
+          BalancedDelimiterTracker T(*this, tok::l_paren);
+          if (T.consumeOpen()) {
+            assert(false && "Not a left paren?");
+            return;
+          }
+          T.skipToEnd();
+
+          Diag(Loc, diag::err_ms_attributes_not_enabled);
+          continue;
+        }
+      }
+
       // In C++, check to see if this is a scope specifier like foo::bar::, if
       // so handle it as such.  This is important for ctor parsing.
       if (getLangOpts().CPlusPlus) {
Index: include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -176,6 +176,9 @@
 def warn_attribute_no_decl : Warning<
   "attribute %0 ignored, because it is not attached to a declaration">, 
   InGroup<IgnoredAttributes>;
+def err_ms_attributes_not_enabled : Error<
+  "'__declspec' attributes are not enabled; use '-fdeclspec' or "
+  "'-fms-extensions' to enable support for __declspec attributes">;
 def err_expected_method_body : Error<"expected method body">;
 def err_declspec_after_virtspec : Error<
   "'%0' qualifier may not appear after the virtual specifier '%1'">;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29868.88107.patch
Type: text/x-patch
Size: 3413 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170211/e677c76b/attachment.bin>