Revert r119838 "Don't warn for empty 'if' body if there is a macro that expands to...

and use a better and more general approach, where NullStmt has a flag to indicate whether it was preceded by an empty macro.

Thanks to Abramo Bagnara for the hint!

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

diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h
index 8726bfb417d63173a0cdbb8b52afc326b6b99aa6..131853265335dac11c2b1f2284049c4e55889d91 100644 (file)
--- a/include/clang/AST/Stmt.h
+++ b/include/clang/AST/Stmt.h
@@ -360,8 +360,16 @@ public:
 ///
 class NullStmt : public Stmt {
   SourceLocation SemiLoc;
+
+  /// \brief Whether the null statement was preceded by an empty macro, e.g:
+  /// @code
+  ///   #define CALL(x)
+  ///   CALL(0);
+  /// @endcode
+  bool LeadingEmptyMacro;
 public:
-  NullStmt(SourceLocation L) : Stmt(NullStmtClass), SemiLoc(L) {}
+  NullStmt(SourceLocation L, bool LeadingEmptyMacro = false)
+    : Stmt(NullStmtClass), SemiLoc(L), LeadingEmptyMacro(LeadingEmptyMacro) {}
 
   /// \brief Build an empty null statement.
   explicit NullStmt(EmptyShell Empty) : Stmt(NullStmtClass, Empty) { }
@@ -369,6 +377,8 @@ public:
   SourceLocation getSemiLoc() const { return SemiLoc; }
   void setSemiLoc(SourceLocation L) { SemiLoc = L; }
 
+  bool hasLeadingEmptyMacro() const { return LeadingEmptyMacro; }
+
   virtual SourceRange getSourceRange() const { return SourceRange(SemiLoc); }
 
   static bool classof(const Stmt *T) {
@@ -379,6 +389,9 @@ public:
   // Iterators
   virtual child_iterator child_begin();
   virtual child_iterator child_end();
+
+  friend class ASTStmtReader;
+  friend class ASTStmtWriter;
 };
 
 /// CompoundStmt - This represents a group of statements like { stmt stmt }.
@@ -651,19 +664,10 @@ class IfStmt : public Stmt {
 
   SourceLocation IfLoc;
   SourceLocation ElseLoc;
-
-  /// \brief True if we have code like:
-  /// @code
-  ///   #define CALL(x)
-  ///   if (condition)
-  ///     CALL(0);
-  /// @endcode
-  bool MacroExpandedInThenStmt;
-
+  
 public:
   IfStmt(ASTContext &C, SourceLocation IL, VarDecl *var, Expr *cond, 
-         Stmt *then, SourceLocation EL = SourceLocation(), Stmt *elsev = 0,
-         bool macroExpandedInThenStmt = false);
+         Stmt *then, SourceLocation EL = SourceLocation(), Stmt *elsev = 0);
   
   /// \brief Build an empty if/then/else statement
   explicit IfStmt(EmptyShell Empty) : Stmt(IfStmtClass, Empty) { }
@@ -695,8 +699,6 @@ public:
   SourceLocation getElseLoc() const { return ElseLoc; }
   void setElseLoc(SourceLocation L) { ElseLoc = L; }
 
-  bool hasMacroExpandedInThenStmt() const { return MacroExpandedInThenStmt; }
-
   virtual SourceRange getSourceRange() const {
     if (SubExprs[ELSE])
       return SourceRange(IfLoc, SubExprs[ELSE]->getLocEnd());
@@ -713,9 +715,6 @@ public:
   // over the initialization expression referenced by the condition variable.
   virtual child_iterator child_begin();
   virtual child_iterator child_end();
-
-  friend class ASTStmtReader;
-  friend class ASTStmtWriter;
 };
 
 /// SwitchStmt - This represents a 'switch' stmt.

diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h
index 2194d6fe620ac2a95b389c47e3ec169cd48766f3..261daed75bcde0bc71d105c3f32a683530da84da 100644 (file)
--- a/include/clang/Lex/Preprocessor.h
+++ b/include/clang/Lex/Preprocessor.h
@@ -47,7 +47,6 @@ class PPCallbacks;
 class CodeCompletionHandler;
 class DirectoryLookup;
 class PreprocessingRecord;
-class PPMacroExpansionTrap;
   
 /// Preprocessor - This object engages in a tight little dance with the lexer to
 /// efficiently preprocess tokens.  Lexers know only about tokens within a
@@ -111,11 +110,6 @@ class Preprocessor {
   /// DisableMacroExpansion - True if macro expansion is disabled.
   bool DisableMacroExpansion : 1;
 
-  /// \brief This is set to true when a macro is expanded.
-  /// Used by PPMacroExpansionTrap.
-  bool MacroExpansionFlag : 1;
-  friend class PPMacroExpansionTrap;
-
   /// \brief Whether we have already loaded macros from the external source.
   mutable bool ReadMacrosFromExternalSource : 1;
 
@@ -1035,17 +1029,6 @@ public:
   virtual bool HandleComment(Preprocessor &PP, SourceRange Comment) = 0;
 };
 
-/// \brief RAII class that determines when any macro expansion has occurred
-/// between the time the instance was created and the time it was
-/// queried.
-class PPMacroExpansionTrap {
-  Preprocessor &PP;
-public:
-  PPMacroExpansionTrap(Preprocessor &PP) : PP(PP) { reset(); }
-  bool hasMacroExpansionOccured() const { return PP.MacroExpansionFlag; }
-  void reset() { PP.MacroExpansionFlag = false; }
-};
-
 }  // end namespace clang
 
 #endif

diff --git a/include/clang/Lex/Token.h b/include/clang/Lex/Token.h
index 954b36ec6d118a22e237db31aaa9a26eb9bf850f..2a190839069328e628e22875cfbb6cdb8a16ddff 100644 (file)
--- a/include/clang/Lex/Token.h
+++ b/include/clang/Lex/Token.h
@@ -76,7 +76,8 @@ public:
     StartOfLine   = 0x01,  // At start of line or only after whitespace.
     LeadingSpace  = 0x02,  // Whitespace exists before this token.
     DisableExpand = 0x04,  // This identifier may never be macro expanded.
-    NeedsCleaning = 0x08   // Contained an escaped newline or trigraph.
+    NeedsCleaning = 0x08,   // Contained an escaped newline or trigraph.
+    LeadingEmptyMacro = 0x10 // Empty macro exists before this token.
   };
 
   tok::TokenKind getKind() const { return (tok::TokenKind)Kind; }
@@ -231,7 +232,13 @@ public:
   /// newlines in it.
   ///
   bool needsCleaning() const { return (Flags & NeedsCleaning) ? true : false; }
-    
+
+  /// \brief Return true if this token has an empty macro before it.
+  ///
+  bool hasLeadingEmptyMacro() const {
+    return (Flags & LeadingEmptyMacro) ? true : false;
+  }
+
 };
 
 /// PPConditionalInfo - Information about the conditional stack (#if directives)

diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index 325b47d1cc6a79ab52528e716ad9775f9c9e1ba9..e94225813e2f3ae88327d491a650ecfe4a9a9e7f 100644 (file)
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -1199,8 +1199,7 @@ private:
   bool ParseParenExprOrCondition(ExprResult &ExprResult,
                                  Decl *&DeclResult,
                                  SourceLocation Loc,
-                                 bool ConvertToBoolean,
-                                 bool *MacroExpandedAfterRParen = 0);
+                                 bool ConvertToBoolean);
   StmtResult ParseIfStatement(AttributeList *Attr);
   StmtResult ParseSwitchStatement(AttributeList *Attr);
   StmtResult ParseWhileStatement(AttributeList *Attr);

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 2f220dbb67a98820d57b86106e74b4f777a1fc5a..9c36ce03473d16b682901977dbcc4c96d3d87d2f 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -1566,7 +1566,8 @@ public:
 
   StmtResult ActOnExprStmt(FullExprArg Expr);
 
-  StmtResult ActOnNullStmt(SourceLocation SemiLoc);
+  StmtResult ActOnNullStmt(SourceLocation SemiLoc,
+                           bool LeadingEmptyMacro = false);
   StmtResult ActOnCompoundStmt(SourceLocation L, SourceLocation R,
                                        MultiStmtArg Elts,
                                        bool isStmtExpr);
@@ -1590,7 +1591,7 @@ public:
                             bool HasUnusedAttr);
   StmtResult ActOnIfStmt(SourceLocation IfLoc,
                                  FullExprArg CondVal, Decl *CondVar,
-                                 Stmt *ThenVal, bool MacroExpandedInThenStmt,
+                                 Stmt *ThenVal,
                                  SourceLocation ElseLoc, Stmt *ElseVal);
   StmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
                                             Expr *Cond,

diff --git a/lib/AST/Stmt.cpp b/lib/AST/Stmt.cpp
index 85e640701bfb5df7c03ce795e04791869470ceb2..acd77beaca4b6cb70b07a01ea8bca31804515f69 100644 (file)
--- a/lib/AST/Stmt.cpp
+++ b/lib/AST/Stmt.cpp
@@ -470,10 +470,8 @@ CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock,
 }
 
 IfStmt::IfStmt(ASTContext &C, SourceLocation IL, VarDecl *var, Expr *cond, 
-               Stmt *then, SourceLocation EL, Stmt *elsev,
-               bool macroExpandedInThenStmt)
-  : Stmt(IfStmtClass), IfLoc(IL), ElseLoc(EL),
-    MacroExpandedInThenStmt(macroExpandedInThenStmt)
+               Stmt *then, SourceLocation EL, Stmt *elsev)
+  : Stmt(IfStmtClass), IfLoc(IL), ElseLoc(EL)
 {
   setConditionVariable(C, var);
   SubExprs[COND] = reinterpret_cast<Stmt*>(cond);

diff --git a/lib/Lex/PPMacroExpansion.cpp b/lib/Lex/PPMacroExpansion.cpp
index 6d2c387d52880a93303fcc0ba23c0cf0ab6b01c5..d3f3db31daa41585e2bfd2df77e858b78bea578a 100644 (file)
--- a/lib/Lex/PPMacroExpansion.cpp
+++ b/lib/Lex/PPMacroExpansion.cpp
@@ -176,7 +176,6 @@ bool Preprocessor::isNextPPTokenLParen() {
 /// expanded as a macro, handle it and return the next token as 'Identifier'.
 bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
                                                  MacroInfo *MI) {
-  MacroExpansionFlag = true;
   if (Callbacks) Callbacks->MacroExpands(Identifier, MI);
 
   // If this is a macro expansion in the "#if !defined(x)" line for the file,
@@ -249,6 +248,7 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
       if (IsAtStartOfLine) Identifier.setFlag(Token::StartOfLine);
       if (HadLeadingSpace) Identifier.setFlag(Token::LeadingSpace);
     }
+    Identifier.setFlag(Token::LeadingEmptyMacro);
     ++NumFastMacroExpanded;
     return false;
 

diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index e3c15680c231f253fc1d1c4316c0881d753065c2..26d2279d31ef1e62ef96cccaadc7a466031202d7 100644 (file)
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -147,8 +147,10 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts, bool OnlyStatement) {
 
   case tok::l_brace:                // C99 6.8.2: compound-statement
     return ParseCompoundStatement(AttrList);
-  case tok::semi:                   // C99 6.8.3p3: expression[opt] ';'
-    return Actions.ActOnNullStmt(ConsumeToken());
+  case tok::semi: {                 // C99 6.8.3p3: expression[opt] ';'
+    bool LeadingEmptyMacro = Tok.hasLeadingEmptyMacro();
+    return Actions.ActOnNullStmt(ConsumeToken(), LeadingEmptyMacro);
+  }
 
   case tok::kw_if:                  // C99 6.8.4.1: if-statement
     return ParseIfStatement(AttrList);
@@ -538,8 +540,7 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
 bool Parser::ParseParenExprOrCondition(ExprResult &ExprResult,
                                        Decl *&DeclResult,
                                        SourceLocation Loc,
-                                       bool ConvertToBoolean,
-                                       bool *MacroExpandedAfterRParen) {
+                                       bool ConvertToBoolean) {
   bool ParseError = false;
   
   SourceLocation LParenLoc = ConsumeParen();
@@ -568,14 +569,7 @@ bool Parser::ParseParenExprOrCondition(ExprResult &ExprResult,
   }
 
   // Otherwise the condition is valid or the rparen is present.
-
-  // Catch a macro expansion after ')'. This is used to know that there is a
-  // macro for 'if' body and not warn for empty body if the macro is empty.
-  PPMacroExpansionTrap MacroExpansionTrap(PP);
   MatchRHSPunctuation(tok::r_paren, LParenLoc);
-  if (MacroExpandedAfterRParen)
-    *MacroExpandedAfterRParen = MacroExpansionTrap.hasMacroExpansionOccured();
-
   return false;
 }
 
@@ -618,9 +612,7 @@ StmtResult Parser::ParseIfStatement(AttributeList *Attr) {
   // Parse the condition.
   ExprResult CondExp;
   Decl *CondVar = 0;
-  bool MacroExpandedInThenStmt;
-  if (ParseParenExprOrCondition(CondExp, CondVar, IfLoc, true,
-                                &MacroExpandedInThenStmt))
+  if (ParseParenExprOrCondition(CondExp, CondVar, IfLoc, true))
     return StmtError();
 
   FullExprArg FullCondExp(Actions.MakeFullExpr(CondExp.get()));
@@ -704,7 +696,7 @@ StmtResult Parser::ParseIfStatement(AttributeList *Attr) {
     ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
   return Actions.ActOnIfStmt(IfLoc, FullCondExp, CondVar, ThenStmt.get(),
-                             MacroExpandedInThenStmt, ElseLoc, ElseStmt.get());
+                             ElseLoc, ElseStmt.get());
 }
 
 /// ParseSwitchStatement

diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index c6194edac3a4b30149b3be4eafa03103abfbc7a3..68b7a8c163b2bd03e0841f39c984d93b0d4ff0aa 100644 (file)
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -42,8 +42,8 @@ StmtResult Sema::ActOnExprStmt(FullExprArg expr) {
 }
 
 
-StmtResult Sema::ActOnNullStmt(SourceLocation SemiLoc) {
-  return Owned(new (Context) NullStmt(SemiLoc));
+StmtResult Sema::ActOnNullStmt(SourceLocation SemiLoc, bool LeadingEmptyMacro) {
+  return Owned(new (Context) NullStmt(SemiLoc, LeadingEmptyMacro));
 }
 
 StmtResult Sema::ActOnDeclStmt(DeclGroupPtrTy dg,
@@ -282,8 +282,8 @@ Sema::ActOnLabelStmt(SourceLocation IdentLoc, IdentifierInfo *II,
 
 StmtResult
 Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar,
-                  Stmt *thenStmt, bool MacroExpandedInThenStmt,
-                  SourceLocation ElseLoc, Stmt *elseStmt) {
+                  Stmt *thenStmt, SourceLocation ElseLoc,
+                  Stmt *elseStmt) {
   ExprResult CondResult(CondVal.release());
 
   VarDecl *ConditionVar = 0;
@@ -312,15 +312,14 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar,
       // if (condition)
       //   CALL(0);
       //
-      if (!MacroExpandedInThenStmt)
+      if (!stmt->hasLeadingEmptyMacro())
         Diag(stmt->getSemiLoc(), diag::warn_empty_if_body);
   }
 
   DiagnoseUnusedExprResult(elseStmt);
 
   return Owned(new (Context) IfStmt(Context, IfLoc, ConditionVar, ConditionExpr, 
-                                    thenStmt, ElseLoc, elseStmt,
-                                    MacroExpandedInThenStmt));
+                                    thenStmt, ElseLoc, elseStmt));
 }
 
 /// ConvertIntegerToTypeWarnOnOverflow - Convert the specified APInt to have

diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
index 3ae4e5c5f49cbd6709f47d65220f0382c10e0719..807346c4c57a52a9657158159529f182ac8e99fa 100644 (file)
--- a/lib/Sema/TreeTransform.h
+++ b/lib/Sema/TreeTransform.h
@@ -772,11 +772,9 @@ public:
   /// By default, performs semantic analysis to build the new statement.
   /// Subclasses may override this routine to provide different behavior.
   StmtResult RebuildIfStmt(SourceLocation IfLoc, Sema::FullExprArg Cond,
-                                 VarDecl *CondVar, Stmt *Then,
-                                 bool MacroExpandedInThenStmt,
+                                 VarDecl *CondVar, Stmt *Then, 
                                  SourceLocation ElseLoc, Stmt *Else) {
-    return getSema().ActOnIfStmt(IfLoc, Cond, CondVar, Then,
-                                 MacroExpandedInThenStmt, ElseLoc, Else);
+    return getSema().ActOnIfStmt(IfLoc, Cond, CondVar, Then, ElseLoc, Else);
   }
 
   /// \brief Start building a new switch statement.
@@ -3694,7 +3692,7 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
     return SemaRef.Owned(S);
 
   return getDerived().RebuildIfStmt(S->getIfLoc(), FullCond, ConditionVar,
-                                    Then.get(), S->hasMacroExpandedInThenStmt(),
+                                    Then.get(),
                                     S->getElseLoc(), Else.get());
 }
 

diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp
index a7b42cd3fcfded6e17bf61ec68b3ee6f62e08364..5aba1f8ada7b54d52d229405235e86adde5bcb45 100644 (file)
--- a/lib/Serialization/ASTReaderStmt.cpp
+++ b/lib/Serialization/ASTReaderStmt.cpp
@@ -202,6 +202,7 @@ void ASTStmtReader::VisitStmt(Stmt *S) {
 void ASTStmtReader::VisitNullStmt(NullStmt *S) {
   VisitStmt(S);
   S->setSemiLoc(ReadSourceLocation(Record, Idx));
+  S->LeadingEmptyMacro = Record[Idx++];
 }
 
 void ASTStmtReader::VisitCompoundStmt(CompoundStmt *S) {
@@ -256,7 +257,6 @@ void ASTStmtReader::VisitIfStmt(IfStmt *S) {
   S->setElse(Reader.ReadSubStmt());
   S->setIfLoc(ReadSourceLocation(Record, Idx));
   S->setElseLoc(ReadSourceLocation(Record, Idx));
-  S->MacroExpandedInThenStmt = Record[Idx++];
 }
 
 void ASTStmtReader::VisitSwitchStmt(SwitchStmt *S) {

diff --git a/lib/Serialization/ASTWriterStmt.cpp b/lib/Serialization/ASTWriterStmt.cpp
index a59b772da4e98cb301abf5d5d23c70f3fd2d67e2..4d2fb8a0301fc45ef112d3191e23f664958c0a50 100644 (file)
--- a/lib/Serialization/ASTWriterStmt.cpp
+++ b/lib/Serialization/ASTWriterStmt.cpp
@@ -171,6 +171,7 @@ void ASTStmtWriter::VisitStmt(Stmt *S) {
 void ASTStmtWriter::VisitNullStmt(NullStmt *S) {
   VisitStmt(S);
   Writer.AddSourceLocation(S->getSemiLoc(), Record);
+  Record.push_back(S->LeadingEmptyMacro);
   Code = serialization::STMT_NULL;
 }
 
@@ -228,7 +229,6 @@ void ASTStmtWriter::VisitIfStmt(IfStmt *S) {
   Writer.AddStmt(S->getElse());
   Writer.AddSourceLocation(S->getIfLoc(), Record);
   Writer.AddSourceLocation(S->getElseLoc(), Record);
-  Record.push_back(S->MacroExpandedInThenStmt);
   Code = serialization::STMT_IF;
 }