[cfe-commits] r57790 - in /cfe/trunk: include/clang/Basic/DiagnosticKinds.def include/clang/Parse/Parser.h lib/Parse/ParseDecl.cpp test/Parser/attributes.c

Chris Lattner sabre at nondot.org
Sun Oct 19 19:05:46 PDT 2008


Author: lattner
Date: Sun Oct 19 21:05:46 2008
New Revision: 57790

URL: http://llvm.org/viewvc/llvm-project?rev=57790&view=rev
Log:
Fix a parser bug where we let attributes interfere with our disambiguation
of whether a '(' was a grouping paren or the start of a function declarator.
This is PR2796.

Now we eat the attribute before deciding whether the paren is grouping or
not, then apply it to the resultant decl or to the first argument as needed.

One somewhat surprising aspect of this is that attributes interact with
implicit int in cases like this:

void a(x, y) // k&r style function
void b(__attribute__(()) x, y); // function with two implicit int arguments
void c(x, __attribute__(()) y); // error, can't have attr in identifier list.

Fun stuff.


Modified:
    cfe/trunk/include/clang/Basic/DiagnosticKinds.def
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/lib/Parse/ParseDecl.cpp
    cfe/trunk/test/Parser/attributes.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticKinds.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticKinds.def?rev=57790&r1=57789&r2=57790&view=diff

==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticKinds.def (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticKinds.def Sun Oct 19 21:05:46 2008
@@ -246,8 +246,7 @@
 //===----------------------------------------------------------------------===//
 // Parser Diagnostics
 //===----------------------------------------------------------------------===//
-DIAG(w_type_defaults_to_int, WARNING,
-     "type defaults to 'int'")
+
 DIAG(w_no_declarators, WARNING,
      "declaration does not declare anything")
 DIAG(w_asm_qualifier_ignored, WARNING,
@@ -757,6 +756,8 @@
      "'iboutlet' attribute can only be applied to instance variables")
 
 // Function Parameter Semantic Analysis.
+DIAG(err_argument_required_after_attribute, ERROR,
+     "argument required after attribute")
 DIAG(err_param_with_void_type, ERROR,
      "argument may not have 'void' type")
 DIAG(err_void_only_param, ERROR,

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=57790&r1=57789&r2=57790&view=diff

==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Sun Oct 19 21:05:46 2008
@@ -692,7 +692,9 @@
   void ParseTypeQualifierListOpt(DeclSpec &DS);
   void ParseDirectDeclarator(Declarator &D);
   void ParseParenDeclarator(Declarator &D);
-  void ParseFunctionDeclarator(SourceLocation LParenLoc, Declarator &D);
+  void ParseFunctionDeclarator(SourceLocation LParenLoc, Declarator &D,
+                               AttributeList *AttrList = 0,
+                               bool RequiresArg = false);
   void ParseFunctionDeclaratorIdentifierList(SourceLocation LParenLoc,
                                              Declarator &D);
   void ParseBracketDeclarator(Declarator &D);

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=57790&r1=57789&r2=57790&view=diff

==============================================================================
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Sun Oct 19 21:05:46 2008
@@ -1227,15 +1227,16 @@
   
   while (1) {
     if (Tok.is(tok::l_paren)) {
-      // When not in file scope, warn for ambiguous function declarators, just
-      // in case the author intended it as a variable definition.
-      bool warnIfAmbiguous = D.getContext() != Declarator::FileContext;
       // The paren may be part of a C++ direct initializer, eg. "int x(1);".
       // In such a case, check if we actually have a function declarator; if it
       // is not, the declarator has been fully parsed.
-      if (getLang().CPlusPlus && D.mayBeFollowedByCXXDirectInit() &&
-          !isCXXFunctionDeclarator(warnIfAmbiguous))
-        break;
+      if (getLang().CPlusPlus && D.mayBeFollowedByCXXDirectInit()) {
+        // When not in file scope, warn for ambiguous function declarators, just
+        // in case the author intended it as a variable definition.
+        bool warnIfAmbiguous = D.getContext() != Declarator::FileContext;
+        if (!isCXXFunctionDeclarator(warnIfAmbiguous))
+          break;
+      }
       ParseFunctionDeclarator(ConsumeParen(), D);
     } else if (Tok.is(tok::l_square)) {
       ParseBracketDeclarator(D);
@@ -1253,11 +1254,35 @@
 ///       direct-declarator:
 ///         '(' declarator ')'
 /// [GNU]   '(' attributes declarator ')'
+///         direct-declarator '(' parameter-type-list ')'
+///         direct-declarator '(' identifier-list[opt] ')'
+/// [GNU]   direct-declarator '(' parameter-forward-declarations
+///                    parameter-type-list[opt] ')'
 ///
 void Parser::ParseParenDeclarator(Declarator &D) {
   SourceLocation StartLoc = ConsumeParen();
   assert(!D.isPastIdentifier() && "Should be called before passing identifier");
   
+  // Eat any attributes before we look at whether this is a grouping or function
+  // declarator paren.  If this is a grouping paren, the attribute applies to
+  // the type being built up, for example:
+  //     int (__attribute__(()) *x)(long y)
+  // If this ends up not being a grouping paren, the attribute applies to the
+  // first argument, for example:
+  //     int (__attribute__(()) int x)
+  // In either case, we need to eat any attributes to be able to determine what
+  // sort of paren this is.
+  //
+  AttributeList *AttrList = 0;
+  bool RequiresArg = false;
+  if (Tok.is(tok::kw___attribute)) {
+    AttrList = ParseAttributes();
+    
+    // We require that the argument list (if this is a non-grouping paren) be
+    // present even if the attribute list was empty.
+    RequiresArg = true;
+  }
+  
   // If we haven't past the identifier yet (or where the identifier would be
   // stored, if this is an abstract declarator), then this is probably just
   // grouping parens. However, if this could be an abstract-declarator, then
@@ -1285,10 +1310,9 @@
   if (isGrouping) {
     bool hadGroupingParens = D.hasGroupingParens();
     D.setGroupingParens(true);
+    if (AttrList)
+      D.AddAttributes(AttrList);
 
-    if (Tok.is(tok::kw___attribute))
-      D.AddAttributes(ParseAttributes());
-    
     ParseDeclaratorInternal(D);
     // Match the ')'.
     MatchRHSPunctuation(tok::r_paren, StartLoc);
@@ -1299,17 +1323,23 @@
   
   // Okay, if this wasn't a grouping paren, it must be the start of a function
   // argument list.  Recognize that this declarator will never have an
-  // identifier (and remember where it would have been), then fall through to
-  // the handling of argument lists.
+  // identifier (and remember where it would have been), then call into
+  // ParseFunctionDeclarator to handle of argument list.
   D.SetIdentifier(0, Tok.getLocation());
 
-  ParseFunctionDeclarator(StartLoc, D); 
+  ParseFunctionDeclarator(StartLoc, D, AttrList, RequiresArg);
 }
 
 /// ParseFunctionDeclarator - We are after the identifier and have parsed the
 /// declarator D up to a paren, which indicates that we are parsing function
 /// arguments.
 ///
+/// If AttrList is non-null, then the caller parsed those arguments immediately
+/// after the open paren - they should be considered to be the first argument of
+/// a parameter.  If RequiresArg is true, then the first argument of the
+/// function is required to be present and required to not be an identifier
+/// list.
+///
 /// This method also handles this portion of the grammar:
 ///       parameter-type-list: [C99 6.7.5]
 ///         parameter-list
@@ -1328,14 +1358,19 @@
 ///           '=' assignment-expression
 /// [GNU]   declaration-specifiers abstract-declarator[opt] attributes
 ///
-void Parser::ParseFunctionDeclarator(SourceLocation LParenLoc, Declarator &D) {
+void Parser::ParseFunctionDeclarator(SourceLocation LParenLoc, Declarator &D,
+                                     AttributeList *AttrList,
+                                     bool RequiresArg) {
   // lparen is already consumed!
   assert(D.isPastIdentifier() && "Should not call before identifier!");
   
-  // Okay, this is the parameter list of a function definition, or it is an
-  // identifier list of a K&R-style function.
-  
+  // This parameter list may be empty.
   if (Tok.is(tok::r_paren)) {
+    if (RequiresArg) {
+      Diag(Tok.getLocation(), diag::err_argument_required_after_attribute);
+      delete AttrList;
+    }
+    
     // Remember that we parsed a function type, and remember the attributes.
     // int() -> no prototype, no '...'.
     D.AddTypeInfo(DeclaratorChunk::getFunction(/*prototype*/ false,
@@ -1344,10 +1379,19 @@
     
     ConsumeParen();  // Eat the closing ')'.
     return;
-  } else if (Tok.is(tok::identifier) &&
-             // K&R identifier lists can't have typedefs as identifiers, per
-             // C99 6.7.5.3p11.
-             !Actions.isTypeName(*Tok.getIdentifierInfo(), CurScope)) {
+  } 
+  
+  // Alternatively, this parameter list may be an identifier list form for a
+  // K&R-style function:  void foo(a,b,c)
+  if (Tok.is(tok::identifier) &&
+      // K&R identifier lists can't have typedefs as identifiers, per
+      // C99 6.7.5.3p11.
+      !Actions.isTypeName(*Tok.getIdentifierInfo(), CurScope)) {
+    if (RequiresArg) {
+      Diag(Tok.getLocation(), diag::err_argument_required_after_attribute);
+      delete AttrList;
+    }
+    
     // Identifier list.  Note that '(' identifier-list ')' is only allowed for
     // normal declarators, not for abstract-declarators.
     return ParseFunctionDeclaratorIdentifierList(LParenLoc, D);
@@ -1383,6 +1427,12 @@
     
     // Parse the declaration-specifiers.
     DeclSpec DS;
+
+    // If the caller parsed attributes for the first argument, add them now.
+    if (AttrList) {
+      DS.AddAttributes(AttrList);
+      AttrList = 0;  // Only apply the attributes to the first parameter.
+    }
     ParseDeclarationSpecifiers(DS);
 
     // Parse the declarator.  This is "PrototypeContext", because we must

Modified: cfe/trunk/test/Parser/attributes.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/attributes.c?rev=57790&r1=57789&r2=57790&view=diff

==============================================================================
--- cfe/trunk/test/Parser/attributes.c (original)
+++ cfe/trunk/test/Parser/attributes.c Sun Oct 19 21:05:46 2008
@@ -1,6 +1,45 @@
-// RUN: clang -fsyntax-only -verify %s -pedantic
+// RUN: clang -fsyntax-only -verify %s -pedantic -std=c99
 
-static __inline void __attribute__((__always_inline__, __nodebug__)) // expected-warning {{extension used}}
-foo (void)
-{
+int __attribute__(()) x;  // expected-warning {{extension used}}
+
+// Hide __attribute__ behind a macro, to silence extension warnings about
+// "__attribute__ being an extension".
+#define attribute __attribute__
+
+__inline void attribute((__always_inline__, __nodebug__))
+foo(void) {
 }
+
+
+attribute(()) y;   // expected-warning {{defaults to 'int'}}
+
+// PR2796
+int (attribute(()) *z)(long y);
+
+
+void f1(attribute(()) int x);
+
+int f2(y, attribute(()) x);     // expected-error {{expected identifier}}
+
+// This is parsed as a normal argument list (with two args that are implicit
+// int) because the attribute is a declspec.
+void f3(attribute(()) x,  // expected-warning {{defaults to 'int'}}
+        y);               // expected-warning {{defaults to 'int'}}
+
+void f4(attribute(()));   // expected-error {{expected parameter declarator}}
+
+
+// This is ok, the attribute applies to the pointer.
+int baz(int (attribute(()) *x)(long y));
+
+void g1(void (*f1)(attribute(()) int x));
+void g2(int (*f2)(y, attribute(()) x));    // expected-error {{expected identifier}}
+void g3(void (*f3)(attribute(()) x, int y));  // expected-warning {{defaults to 'int'}}
+void g4(void (*f4)(attribute(())));  // expected-error {{expected parameter declarator}}
+
+
+void (*h1)(void (*f1)(attribute(()) int x));
+void (*h2)(int (*f2)(y, attribute(()) x));    // expected-error {{expected identifier}}
+
+void (*h3)(void (*f3)(attribute(()) x));   // expected-warning {{defaults to 'int'}}
+void (*h4)(void (*f4)(attribute(())));  // expected-error {{expected parameter declarator}}





More information about the cfe-commits mailing list