From: Ted Kremenek Date: Mon, 3 Aug 2009 23:24:57 +0000 (+0000) Subject: Per advice that Doug Gregor gave me several months ago, clean up the X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7a02a3733cdd2ca672902d869fda4ef2e3f05052;p=clang 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: [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 8f5378d998..5a7fbf4ff5 100644 --- 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 bfe90d98b7..c4941c1f7e 100644 --- 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 58c729aef2..68b10934b1 100644 --- 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 Ex; + llvm::SmallVector 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 0dbb9f81d1..2a77f21a2d 100644 --- 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 1bf8444c42..6622d53659 100644 --- 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(Ex)) { - ErrorExpr = Ex; - break; - } - Decl *d = cast(Ex)->getDecl();; - - if (!isa(d) || !cast(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(ND) || !cast(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 fe8bf8608b..8b94989626 100644 --- 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'}} +} +