Per advice that Doug Gregor gave me several months ago, clean up the

implementation of '#pragma unused' by not constructing intermediate
DeclRefExprs, but instead do the name lookup directly.  The
implementation is greatly simplified.

Along the way, degrade '#pragma unused(undeclaredvariable)' to a
warning instead of being a hard error.  This implements:

<rdar://problem/6761874> [sema] allow #pragma unused to reference undefined variable (with warning)

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@78019 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 8f5378d998544cad81217ebae5bd8a0cb5899ed4..5a7fbf4ff5712f6e0689fe286434db3f27ad8a76 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -167,6 +167,8 @@ def warn_pragma_pack_pop_identifer_and_alignment : Warning<
   "specifying both a name and alignment to 'pop' is undefined">;
 def warn_pragma_pack_pop_failed : Warning<"#pragma pack(pop, ...) failed: %0">;
 
+def warn_pragma_unused_undeclared_var : Warning<
+  "undeclared variable %0 used as an argument for '#pragma unused'">;
 def warn_pragma_unused_expected_localvar : Warning<
   "only local variables can be arguments to '#pragma unused'">;
 def err_unsupported_pragma_weak : Error<

diff --git a/include/clang/Parse/Action.h b/include/clang/Parse/Action.h
index bfe90d98b79b3d927a2c6750ab2e8cff798064b7..c4941c1f7e50fe047500d89d81e4a927d67b2c55 100644 (file)
--- a/include/clang/Parse/Action.h
+++ b/include/clang/Parse/Action.h
@@ -1904,7 +1904,8 @@ public:
   }
   
   /// ActOnPragmaUnused - Called on well formed #pragma unused(...).
-  virtual void ActOnPragmaUnused(ExprTy **Exprs, unsigned NumExprs,
+  virtual void ActOnPragmaUnused(const Token *Identifiers,
+                                 unsigned NumIdentifiers, Scope *CurScope,
                                  SourceLocation PragmaLoc, 
                                  SourceLocation LParenLoc,
                                  SourceLocation RParenLoc) {

diff --git a/lib/Parse/ParsePragma.cpp b/lib/Parse/ParsePragma.cpp
index 58c729aef29e6961d9d1896a9e0179fa29fca761..68b10934b15b527cf3f1c03e8f50683967a98d87 100644 (file)
--- a/lib/Parse/ParsePragma.cpp
+++ b/lib/Parse/ParsePragma.cpp
@@ -126,7 +126,7 @@ void PragmaUnusedHandler::HandlePragma(Preprocessor &PP, Token &UnusedTok) {
   SourceLocation LParenLoc = Tok.getLocation();
   
   // Lex the declaration reference(s).
-  llvm::SmallVector<Action::ExprTy*, 5> Ex;
+  llvm::SmallVector<Token, 5> Identifiers;
   SourceLocation RParenLoc;
   bool LexID = true;
   
@@ -134,27 +134,13 @@ void PragmaUnusedHandler::HandlePragma(Preprocessor &PP, Token &UnusedTok) {
     PP.Lex(Tok);
     
     if (LexID) {
-      if (Tok.is(tok::identifier)) {            
-        Action::OwningExprResult Name = 
-          Actions.ActOnIdentifierExpr(parser.CurScope, Tok.getLocation(),
-                                      *Tok.getIdentifierInfo(), false);
-      
-        if (Name.isInvalid()) {
-          if (!Ex.empty())
-            Action::MultiExprArg Release(Actions, &Ex[0], Ex.size());
-          return;
-        }
-        
-        Ex.push_back(Name.release());        
+      if (Tok.is(tok::identifier)) {  
+        Identifiers.push_back(Tok);
         LexID = false;
         continue;
       }
 
-      // Illegal token! Release the parsed expressions (if any) and emit
-      // a warning.
-      if (!Ex.empty())
-        Action::MultiExprArg Release(Actions, &Ex[0], Ex.size());
-      
+      // Illegal token!
       PP.Diag(Tok.getLocation(), diag::warn_pragma_unused_expected_var);
       return;
     }
@@ -170,11 +156,7 @@ void PragmaUnusedHandler::HandlePragma(Preprocessor &PP, Token &UnusedTok) {
       break;
     }
     
-    // Illegal token! Release the parsed expressions (if any) and emit
-    // a warning.
-    if (!Ex.empty())
-      Action::MultiExprArg Release(Actions, &Ex[0], Ex.size());
-    
+    // Illegal token!
     PP.Diag(Tok.getLocation(), diag::warn_pragma_unused_expected_punc);
     return;
   }
@@ -188,10 +170,11 @@ void PragmaUnusedHandler::HandlePragma(Preprocessor &PP, Token &UnusedTok) {
 
   // Verify that we have a location for the right parenthesis.
   assert(RParenLoc.isValid() && "Valid '#pragma unused' must have ')'");
-  assert(!Ex.empty() && "Valid '#pragma unused' must have arguments");
+  assert(!Identifiers.empty() && "Valid '#pragma unused' must have arguments");
 
   // Perform the action to handle the pragma.    
-  Actions.ActOnPragmaUnused(&Ex[0], Ex.size(), UnusedLoc, LParenLoc, RParenLoc);
+  Actions.ActOnPragmaUnused(Identifiers.data(), Identifiers.size(),
+                            parser.CurScope, UnusedLoc, LParenLoc, RParenLoc);
 }
 
 // #pragma weak identifier

diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index 0dbb9f81d1e69c01850266b3cf6d510074fa1d1f..2a77f21a2d4726ffb1cfd9032a9f1e76587c27ad 100644 (file)
--- a/lib/Sema/Sema.h
+++ b/lib/Sema/Sema.h
@@ -2953,7 +2953,8 @@ public:
                                SourceLocation RParenLoc);
   
   /// ActOnPragmaUnused - Called on well-formed '#pragma unused'.
-  virtual void ActOnPragmaUnused(ExprTy **Exprs, unsigned NumExprs,
+  virtual void ActOnPragmaUnused(const Token *Identifiers,
+                                 unsigned NumIdentifiers, Scope *curScope,
                                  SourceLocation PragmaLoc, 
                                  SourceLocation LParenLoc,
                                  SourceLocation RParenLoc);

diff --git a/lib/Sema/SemaAttr.cpp b/lib/Sema/SemaAttr.cpp
index 1bf8444c42b74d2da36fed67d8ebf266413ead9b..6622d53659efea13b8c042c20541c26a0c4d6e58 100644 (file)
--- a/lib/Sema/SemaAttr.cpp
+++ b/lib/Sema/SemaAttr.cpp
@@ -170,42 +170,35 @@ void Sema::ActOnPragmaPack(PragmaPackKind Kind, IdentifierInfo *Name,
   }
 }
 
-void Sema::ActOnPragmaUnused(ExprTy **Exprs, unsigned NumExprs,
+void Sema::ActOnPragmaUnused(const Token *Identifiers, unsigned NumIdentifiers,
+                             Scope *curScope,
                              SourceLocation PragmaLoc,
                              SourceLocation LParenLoc,
                              SourceLocation RParenLoc) {
-  
-  // Verify that all of the expressions are valid before
-  // modifying the attributes of any referenced decl.
-  Expr *ErrorExpr = 0;
-  
-  for (unsigned i = 0; i < NumExprs; ++i) {  
-    Expr *Ex = (Expr*) Exprs[i];
-    if (!isa<DeclRefExpr>(Ex)) {
-      ErrorExpr = Ex;
-      break;
-    }
 
-    Decl *d = cast<DeclRefExpr>(Ex)->getDecl();;
-
-    if (!isa<VarDecl>(d) || !cast<VarDecl>(d)->hasLocalStorage()) {
-      ErrorExpr = Ex;
-      break;
+  for (unsigned i = 0; i < NumIdentifiers; ++i) {
+    const Token &Tok = Identifiers[i];
+    IdentifierInfo *Name = Tok.getIdentifierInfo();    
+    const LookupResult &Lookup = LookupParsedName(curScope, NULL, Name,
+                                                  LookupOrdinaryName,
+                                                  false, true,
+                                                  Tok.getLocation());
+    // FIXME: Handle Lookup.isAmbiguous?
+
+    NamedDecl *ND = Lookup.getAsDecl();
+
+    if (!ND) {      
+      Diag(PragmaLoc, diag::warn_pragma_unused_undeclared_var)
+        << Name << SourceRange(Tok.getLocation());
+      continue;
     }
-  }
-  
-  // Delete the expressions if we encountered any error.
-  if (ErrorExpr) {
-    Diag(ErrorExpr->getLocStart(), diag::warn_pragma_unused_expected_localvar);
-    for (unsigned i = 0; i < NumExprs; ++i)
-      ((Expr*) Exprs[i])->Destroy(Context);    
-    return;
-  }
       
-  // Otherwise, add the 'unused' attribute to each referenced declaration.
-  for (unsigned i = 0; i < NumExprs; ++i) {
-    DeclRefExpr *DR = (DeclRefExpr*) Exprs[i];
-    DR->getDecl()->addAttr(::new (Context) UnusedAttr());
-    DR->Destroy(Context);
+    if (!isa<VarDecl>(ND) || !cast<VarDecl>(ND)->hasLocalStorage()) {
+      Diag(PragmaLoc, diag::warn_pragma_unused_expected_localvar)
+        << Name << SourceRange(Tok.getLocation());
+      continue;
+    }
+    
+    ND->addAttr(::new (Context) UnusedAttr());
   }
 }

diff --git a/test/Sema/pragma-unused.c b/test/Sema/pragma-unused.c
index fe8bf8608bc351e4d52c5c2cccf305b754ec8909..8b94989626469f9bc5a7ccf9e00628b12d446970 100644 (file)
--- a/test/Sema/pragma-unused.c
+++ b/test/Sema/pragma-unused.c
@@ -16,7 +16,7 @@ void f2(void) {
 }
 
 void f3(void) {
-  #pragma unused(x) // expected-error{{use of undeclared identifier 'x'}}
+  #pragma unused(x) // expected-warning{{undeclared variable 'x' used as an argument for '#pragma unused'}}
 }
 
 void f4(void) {
@@ -26,7 +26,7 @@ void f4(void) {
 
 int k;
 void f5(void) {
-  #pragma unused(k) // expected-warning{{only local variables can be arguments to '#pragma unused' - ignored}}
+  #pragma unused(k) // expected-warning{{only local variables can be arguments to '#pragma unused'}}
 }
 
 void f6(void) {
@@ -36,3 +36,8 @@ void f6(void) {
   }
 }
 
+void f7() {
+  int y;
+  #pragma unused(undeclared, undefined, y) // expected-warning{{undeclared variable 'undeclared' used as an argument for '#pragma unused'}} expected-warning{{undeclared variable 'undefined' used as an argument for '#pragma unused'}}
+}
+