[cfe-commits] r67569 - in /cfe/trunk: include/clang/Basic/DiagnosticParseKinds.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Parse/Action.h include/clang/Parse/Parser.h lib/Parse/ParsePragma.cpp lib/Parse/ParsePragma.h lib/Parse/Parser.cpp lib/Sema/Sema.h lib/Sema/SemaAttr.cpp test/Sema/pragma-unused.c

[Douglas Gregor]

On Mar 23, 2009, at 3:28 PM, Ted Kremenek wrote:

> Author: kremenek
> Date: Mon Mar 23 17:28:25 2009
> New Revision: 67569
>
> URL: http://llvm.org/viewvc/llvm-project?rev=67569&view=rev
> Log:
> Implement '#pragma unused'.

Cool. Some comments below.

> Modified: cfe/trunk/include/clang/Parse/Action.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Action.h?rev=67569&r1=67568&r2=67569&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/include/clang/Parse/Action.h (original)
> +++ cfe/trunk/include/clang/Parse/Action.h Mon Mar 23 17:28:25 2009
> @@ -1483,6 +1483,14 @@
>                                SourceLocation RParenLoc) {
>     return;
>   }
> +
> +  /// ActOnPragmaPack - Called on well formed #pragma pack(...).
> +  virtual void ActOnPragmaUnused(ExprTy **Exprs, unsigned NumExprs,
> +                                 SourceLocation PragmaLoc,
> +                                 SourceLocation LParenLoc,
> +                                 SourceLocation RParenLoc) {
> +    return;
> +  }

The comment here refers to #pragma pack, not #pragma unused.


> +// #pragma unused(identifier)
> +void PragmaUnusedHandler::HandlePragma(Preprocessor &PP, Token  
> &UnusedTok) {
> +  // FIXME: Should we be expanding macros here? My guess is no.
> +  SourceLocation UnusedLoc = UnusedTok.getLocation();
> +
> +  // Lex the left '('.
> +  Token Tok;
> +  PP.Lex(Tok);
> +  if (Tok.isNot(tok::l_paren)) {
> +    PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen)  
> << "unused";
> +    return;
> +  }
> +  SourceLocation LParenLoc = Tok.getLocation();
> +
> +  // Lex the declaration reference(s).
> +  llvm::SmallVector<Action::ExprTy*, 5> Ex;
> +  SourceLocation RParenLoc;
> +  bool LexID = true;
> +
> +  while (true) {
> +    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());
> +        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());
> +
> +      PP.Diag(Tok.getLocation(),  
> diag::warn_pragma_unused_expected_var);
> +      return;
> +    }
> +
> +    // We are execting a ')' or a ','.
> +    if (Tok.is(tok::comma)) {
> +      LexID = true;
> +      continue;
> +    }
> +
> +    if (Tok.is(tok::r_paren)) {
> +      RParenLoc = Tok.getLocation();
> +      break;
> +    }
> +
> +    // Illegal token! Release the parsed expressions (if any) and  
> emit
> +    // a warning.
> +    if (!Ex.empty())
> +      Action::MultiExprArg Release(Actions, &Ex[0], Ex.size());
> +
> +    PP.Diag(Tok.getLocation(),  
> diag::warn_pragma_unused_expected_punc);
> +    return;
> +  }
> +
> +  // 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");
> +
> +  // Perform the action to handle the pragma.
> +  Actions.ActOnPragmaUnused(&Ex[0], Ex.size(), UnusedLoc,  
> LParenLoc, RParenLoc);
> +}

We're going through some extra work here. For each name, we call  
ActOnIdentifierExpr to build a DeclRefExpr, then we have to reverse- 
engineer the DeclRefExpr in ActOnPragmaUnused. Perhaps it would be  
easier to pass arrays of identifiers and locations into  
ActOnPragmaUnused, and skip the interim representation as a  
DeclRefExpr? ActOnPragmaUnused will, of course, need to receive the  
scope so that it can perform lookup for those identifiers.


> Modified: cfe/trunk/lib/Sema/SemaAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAttr.cpp?rev=67569&r1=67568&r2=67569&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Sema/SemaAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaAttr.cpp Mon Mar 23 17:28:25 2009
> @@ -170,3 +170,41 @@
>   }
> }
>
> +void Sema::ActOnPragmaUnused(ExprTy **Exprs, unsigned NumExprs,
> +                             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;
> +    }
> +  }
> +
> +  // 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());
> +  }
> +}

Much of this code could be simplified if we just performed the lookup  
of these variables here. Also, aren't we leaking the DeclRefExprs in  
the case where the #pragma unused is well-formed?

	- Doug