Loosen -Wempty-body warning.

[rnk]

Summary

Do not show it when if or else come from macros.
E.g.,

#define USED(A) if (A); else
#define SOME_IF(A) if (A)

void test() {
  // No warnings are shown in those cases now.
  USED(0);
  SOME_IF(0);
}

[rnk]

Why does passing the rparen location for the if condition fix the warning for IF_ELSE? I assumed that would refer to the else clause semicolon.

[rnk]

I'll go ahead and land this as requested to unblock things. Thanks for the patch, sorry for the trouble!

lib/Sema/SemaChecking.cpp

11818 ↗

(On Diff #123377)

I see, this fixes the else macro case.

[rnk]

• minor tweak

[ilya-biryukov]

In D40185#928845, @rnk wrote:

Why does passing the rparen location for the if condition fix the warning for IF_ELSE? I assumed that would refer to the else clause semicolon.

Using spelling locations fixes the else case, but breaks some of the if cases involving macros (catched by warn-empty-body.cpp test).

cfe/trunk/include/clang/Sema/Sema.h

   /// \brief Given that we had incompatible pointer types in a return
   /// statement, check whether we're in a method with a related result
   /// type, and if so, emit a note describing what happened.
   void EmitRelatedResultTypeNoteForReturn(QualType destType);
 
   class ConditionResult {
     Decl *ConditionVar;
     FullExprArg Condition;
+    SourceLocation RParenLoc;
     bool Invalid;
     bool HasKnownValue;
     bool KnownValue;
 
     friend class Sema;
     ConditionResult(Sema &S, Decl *ConditionVar, FullExprArg Condition,
                     bool IsConstexpr)
         : ConditionVar(ConditionVar), Condition(Condition), Invalid(false),
           HasKnownValue(IsConstexpr && Condition.get() &&
                         !Condition.get()->isValueDependent()),
           KnownValue(HasKnownValue &&
                      !!Condition.get()->EvaluateKnownConstInt(S.Context)) {}
     explicit ConditionResult(bool Invalid)
         : ConditionVar(nullptr), Condition(nullptr), Invalid(Invalid),
           HasKnownValue(false), KnownValue(false) {}
 
   public:
     ConditionResult() : ConditionResult(false) {}
     bool isInvalid() const { return Invalid; }
     std::pair<VarDecl *, Expr *> get() const {
       return std::make_pair(cast_or_null<VarDecl>(ConditionVar),
                             Condition.get());
     }
+
+    void setRParenLoc(SourceLocation Loc) { RParenLoc = Loc; }
+
     llvm::Optional<bool> getKnownValue() const {
       if (!HasKnownValue)
         return None;
       return KnownValue;
     }
   };
   static ConditionResult ConditionError() { return ConditionResult(true); }
 

cfe/trunk/lib/Parse/ParseStmt.cpp

if (Cond.isInvalid() && Tok.isNot(tok::r_paren)) {
     // Skipping may have stopped if it found the containing ')'.  If so, we can
     // continue parsing the if statement.
     if (Tok.isNot(tok::r_paren))
       return true;
   }
 
   // Otherwise the condition is valid or the rparen is present.
   T.consumeClose();
+  Cond.setRParenLoc(T.getCloseLocation());
 
   // Check for extraneous ')'s to catch things like "if (foo())) {".  We know
   // that all callers are looking for a statement after the condition, so ")"
   // isn't valid.
   while (Tok.is(tok::r_paren)) {
     Diag(Tok, diag::err_extraneous_rparen_in_condition)
       << FixItHint::CreateRemoval(Tok.getLocation());
     ConsumeParen();

cfe/trunk/lib/Sema/SemaChecking.cpp

static bool ShouldDiagnoseEmptyStmtBody(const SourceManager &SourceMgr,
   // #define CALL(x)
   // if (condition)
   //   CALL(0);
   if (Body->hasLeadingEmptyMacro())
     return false;
 
   // Get line numbers of statement and body.
   bool StmtLineInvalid;
-  unsigned StmtLine = SourceMgr.getPresumedLineNumber(StmtLoc,
+  unsigned StmtLine = SourceMgr.getSpellingLineNumber(StmtLoc,
                                                       &StmtLineInvalid);
   if (StmtLineInvalid)
     return false;
 
   bool BodyLineInvalid;
   unsigned BodyLine = SourceMgr.getSpellingLineNumber(Body->getSemiLoc(),
                                                       &BodyLineInvalid);
   if (BodyLineInvalid)

cfe/trunk/lib/Sema/SemaStmt.cpp

Sema::ActOnIfStmt(SourceLocation IfLoc, bool IsConstexpr, Stmt *InitStmt,
   Expr *CondExpr = Cond.get().second;
   if (!Diags.isIgnored(diag::warn_comma_operator,
                        CondExpr->getExprLoc()))
     CommaVisitor(*this).Visit(CondExpr);
 
   if (elseStmt)
     DiagnoseEmptyStmtBody(ElseLoc, elseStmt, diag::warn_empty_else_body);
   else
-    DiagnoseEmptyStmtBody(CondExpr->getLocEnd(), thenStmt,
-                          diag::warn_empty_if_body);
+    DiagnoseEmptyStmtBody(Cond.RParenLoc, thenStmt, diag::warn_empty_if_body);
 
   return BuildIfStmt(IfLoc, IsConstexpr, InitStmt, Cond, thenStmt, ElseLoc,
                      elseStmt);
 }
 
 StmtResult Sema::BuildIfStmt(SourceLocation IfLoc, bool IsConstexpr,
                              Stmt *InitStmt, ConditionResult Cond,
                              Stmt *thenStmt, SourceLocation ElseLoc,

cfe/trunk/test/SemaCXX/warn-empty-body.cpp

void test_template_inst(int x) {
   test_template<double>(x);
 }
 
 #define IDENTITY(a) a
 void test7(int x, int y) {
   if (x) IDENTITY(); // no-warning
 }
 
+#define SOME_IF(A) if (A)
+#define IF_ELSE(A) if (A); else
+
+
+void test_macros() {
+  SOME_IF(0);
+  IF_ELSE(0);
+
+  IDENTITY(if (0);) // expected-warning{{if statement has empty body}} expected-note{{separate line}}
+  IDENTITY(if (0); else;) // expected-warning{{else clause has empty body}} expected-note{{separate line}}}
+}