Don't warn for empty 'if' body if there is a macro that expands to nothing, e.g:

  if (condition)
    CALL(0); // empty macro but don't warn for empty body.

Fixes rdar://8436021.

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

diff --git a/include/clang/AST/Stmt.h b/include/clang/AST/Stmt.h
index 6787125863279a0252a382e8ebc8de2c0061d0f6..8726bfb417d63173a0cdbb8b52afc326b6b99aa6 100644 (file)
--- a/include/clang/AST/Stmt.h
+++ b/include/clang/AST/Stmt.h
@@ -651,10 +651,19 @@ 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);
+         Stmt *then, SourceLocation EL = SourceLocation(), Stmt *elsev = 0,
+         bool macroExpandedInThenStmt = false);
   
   /// \brief Build an empty if/then/else statement
   explicit IfStmt(EmptyShell Empty) : Stmt(IfStmtClass, Empty) { }
@@ -686,6 +695,8 @@ 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());
@@ -702,6 +713,9 @@ 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 261daed75bcde0bc71d105c3f32a683530da84da..2194d6fe620ac2a95b389c47e3ec169cd48766f3 100644 (file)
--- a/include/clang/Lex/Preprocessor.h
+++ b/include/clang/Lex/Preprocessor.h
@@ -47,6 +47,7 @@ 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
@@ -110,6 +111,11 @@ 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;
 
@@ -1029,6 +1035,17 @@ 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/Parse/Parser.h b/include/clang/Parse/Parser.h
index e94225813e2f3ae88327d491a650ecfe4a9a9e7f..325b47d1cc6a79ab52528e716ad9775f9c9e1ba9 100644 (file)
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -1199,7 +1199,8 @@ private:
   bool ParseParenExprOrCondition(ExprResult &ExprResult,
                                  Decl *&DeclResult,
                                  SourceLocation Loc,
-                                 bool ConvertToBoolean);
+                                 bool ConvertToBoolean,
+                                 bool *MacroExpandedAfterRParen = 0);
   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 e6cbc329bf5e58ade32a3ceb95cbbcf634f74d1d..8294ffd784a45d3441982905f2c600da4a3bffb3 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -1590,7 +1590,7 @@ public:
                             bool HasUnusedAttr);
   StmtResult ActOnIfStmt(SourceLocation IfLoc,
                                  FullExprArg CondVal, Decl *CondVar,
-                                 Stmt *ThenVal,
+                                 Stmt *ThenVal, bool MacroExpandedInThenStmt,
                                  SourceLocation ElseLoc, Stmt *ElseVal);
   StmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
                                             Expr *Cond,

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

diff --git a/lib/Lex/PPMacroExpansion.cpp b/lib/Lex/PPMacroExpansion.cpp
index 2428f9af4504db0a33236ec214fa49ec67bebd33..6d2c387d52880a93303fcc0ba23c0cf0ab6b01c5 100644 (file)
--- a/lib/Lex/PPMacroExpansion.cpp
+++ b/lib/Lex/PPMacroExpansion.cpp
@@ -176,6 +176,7 @@ 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,

diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index 12444e87bbdacd80d37944ba6a9117749b29b739..e3c15680c231f253fc1d1c4316c0881d753065c2 100644 (file)
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -538,7 +538,8 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
 bool Parser::ParseParenExprOrCondition(ExprResult &ExprResult,
                                        Decl *&DeclResult,
                                        SourceLocation Loc,
-                                       bool ConvertToBoolean) {
+                                       bool ConvertToBoolean,
+                                       bool *MacroExpandedAfterRParen) {
   bool ParseError = false;
   
   SourceLocation LParenLoc = ConsumeParen();
@@ -567,7 +568,14 @@ 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;
 }
 
@@ -610,7 +618,9 @@ StmtResult Parser::ParseIfStatement(AttributeList *Attr) {
   // Parse the condition.
   ExprResult CondExp;
   Decl *CondVar = 0;
-  if (ParseParenExprOrCondition(CondExp, CondVar, IfLoc, true))
+  bool MacroExpandedInThenStmt;
+  if (ParseParenExprOrCondition(CondExp, CondVar, IfLoc, true,
+                                &MacroExpandedInThenStmt))
     return StmtError();
 
   FullExprArg FullCondExp(Actions.MakeFullExpr(CondExp.get()));
@@ -694,7 +704,7 @@ StmtResult Parser::ParseIfStatement(AttributeList *Attr) {
     ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
   return Actions.ActOnIfStmt(IfLoc, FullCondExp, CondVar, ThenStmt.get(),
-                             ElseLoc, ElseStmt.get());
+                             MacroExpandedInThenStmt, ElseLoc, ElseStmt.get());
 }
 
 /// ParseSwitchStatement

diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index a4f1d34aec36a0f82ccfd73eac9eb0de36dff6a2..c6194edac3a4b30149b3be4eafa03103abfbc7a3 100644 (file)
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -282,8 +282,8 @@ Sema::ActOnLabelStmt(SourceLocation IdentLoc, IdentifierInfo *II,
 
 StmtResult
 Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar,
-                  Stmt *thenStmt, SourceLocation ElseLoc,
-                  Stmt *elseStmt) {
+                  Stmt *thenStmt, bool MacroExpandedInThenStmt,
+                  SourceLocation ElseLoc, Stmt *elseStmt) {
   ExprResult CondResult(CondVal.release());
 
   VarDecl *ConditionVar = 0;
@@ -304,17 +304,23 @@ Sema::ActOnIfStmt(SourceLocation IfLoc, FullExprArg CondVal, Decl *CondVar,
   // if (condition);
   //   do_stuff();
   //
-  // NOTE: Do not emit this warning if the body is expanded from a macro.
   if (!elseStmt) {
     if (NullStmt* stmt = dyn_cast<NullStmt>(thenStmt))
-      if (!stmt->getLocStart().isMacroID())
+      // But do not warn if the body is a macro that expands to nothing, e.g:
+      //
+      // #define CALL(x)
+      // if (condition)
+      //   CALL(0);
+      //
+      if (!MacroExpandedInThenStmt)
         Diag(stmt->getSemiLoc(), diag::warn_empty_if_body);
   }
 
   DiagnoseUnusedExprResult(elseStmt);
 
   return Owned(new (Context) IfStmt(Context, IfLoc, ConditionVar, ConditionExpr, 
-                                    thenStmt, ElseLoc, elseStmt));
+                                    thenStmt, ElseLoc, elseStmt,
+                                    MacroExpandedInThenStmt));
 }
 
 /// ConvertIntegerToTypeWarnOnOverflow - Convert the specified APInt to have

diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
index 807346c4c57a52a9657158159529f182ac8e99fa..3ae4e5c5f49cbd6709f47d65220f0382c10e0719 100644 (file)
--- a/lib/Sema/TreeTransform.h
+++ b/lib/Sema/TreeTransform.h
@@ -772,9 +772,11 @@ 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, 
+                                 VarDecl *CondVar, Stmt *Then,
+                                 bool MacroExpandedInThenStmt,
                                  SourceLocation ElseLoc, Stmt *Else) {
-    return getSema().ActOnIfStmt(IfLoc, Cond, CondVar, Then, ElseLoc, Else);
+    return getSema().ActOnIfStmt(IfLoc, Cond, CondVar, Then,
+                                 MacroExpandedInThenStmt, ElseLoc, Else);
   }
 
   /// \brief Start building a new switch statement.
@@ -3692,7 +3694,7 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
     return SemaRef.Owned(S);
 
   return getDerived().RebuildIfStmt(S->getIfLoc(), FullCond, ConditionVar,
-                                    Then.get(),
+                                    Then.get(), S->hasMacroExpandedInThenStmt(),
                                     S->getElseLoc(), Else.get());
 }
 

diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp
index 6d578f616ed396db1cea3cb02da3d82863af9f90..a7b42cd3fcfded6e17bf61ec68b3ee6f62e08364 100644 (file)
--- a/lib/Serialization/ASTReaderStmt.cpp
+++ b/lib/Serialization/ASTReaderStmt.cpp
@@ -256,6 +256,7 @@ 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 495c3d05ffe0d8a6349dd592e7a4600e7ae56d8d..a59b772da4e98cb301abf5d5d23c70f3fd2d67e2 100644 (file)
--- a/lib/Serialization/ASTWriterStmt.cpp
+++ b/lib/Serialization/ASTWriterStmt.cpp
@@ -228,6 +228,7 @@ 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;
 }
 

diff --git a/test/Sema/if-empty-body.c b/test/SemaCXX/if-empty-body.cpp
similarity index 75%
rename from test/Sema/if-empty-body.c
rename to test/SemaCXX/if-empty-body.cpp
index b28c1cdce962f4cb97f5e4d9f73ad6a351951ac8..ec7f89d68e720b1180e2dcc95c6b5b9cce39997c 100644 (file)
--- a/test/Sema/if-empty-body.c
+++ b/test/SemaCXX/if-empty-body.cpp
@@ -16,9 +16,20 @@ void f3() {
 
 // Don't warn about an empty body if is expanded from a macro.
 void f4(int i) {
-  #define BODY ;
+  #define BODY(x)
   if (i == i) // expected-warning{{self-comparison always evaluates to true}}
-    BODY
+    BODY(0);
   #undef BODY
 }
 
+template <typename T>
+void tf() {
+  #define BODY(x)
+  if (0)
+    BODY(0);
+  #undef BODY
+}
+
+void f5() {
+    tf<int>();
+}