commit f371380fc96c67cab15067dea51a55131757d351
author Richard Trieu <rtrieu@google.com> Thu Oct 25 01:08:00 2018 +0000
committer Richard Trieu <rtrieu@google.com> Thu Oct 25 01:08:00 2018 +0000
tree 5d057a41ba8deefc448a4c36f5790bc1b8434fc5
parent a6c6698217d7e20fd5564377d5ba2ab6b3c6f768

[Sema] Fix -Wcomma for C89

There is a small difference in the scope flags for C89 versus the other C/C++
dialects.  This change ensures that the -Wcomma warning won't be duplicated or
issued in the wrong location.  Also, the test case is refactored into C and C++
parts, with the C++ parts guarded by a #ifdef to allow the test to run in both
modes.

https://bugs.llvm.org/show_bug.cgi?id=32370

llvm-svn: 345228

clang/lib/Sema/SemaExpr.cpp

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 98025ca..66194c6 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11309,8 +11309,11 @@
   // The whitelisted locations are the initialization and increment portions
   // of a for loop.  The additional checks are on the condition of
   // if statements, do/while loops, and for loops.
+  // Differences in scope flags for C89 mode requires the extra logic.
   const unsigned ForIncrementFlags =
-      Scope::ControlScope | Scope::ContinueScope | Scope::BreakScope;
+      getLangOpts().C99 || getLangOpts().CPlusPlus
+          ? Scope::ControlScope | Scope::ContinueScope | Scope::BreakScope
+          : Scope::ContinueScope | Scope::BreakScope;
   const unsigned ForInitFlags = Scope::ControlScope | Scope::DeclScope;
   const unsigned ScopeFlags = getCurScope()->getFlags();
   if ((ScopeFlags & ForIncrementFlags) == ForIncrementFlags ||

clang/lib/Sema/SemaStmt.cpp

diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 16ed9ed..7fc89bf 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -551,8 +551,9 @@
         false);
 
   Expr *CondExpr = Cond.get().second;
-  if (!Diags.isIgnored(diag::warn_comma_operator,
-                       CondExpr->getExprLoc()))
+  // Only call the CommaVisitor when not C89 due to differences in scope flags.
+  if ((getLangOpts().C99 || getLangOpts().CPlusPlus) &&
+      !Diags.isIgnored(diag::warn_comma_operator, CondExpr->getExprLoc()))
     CommaVisitor(*this).Visit(CondExpr);
 
   if (!elseStmt)
@@ -1328,6 +1329,11 @@
     return StmtError();
   Cond = CondResult.get();
 
+  // Only call the CommaVisitor for C89 due to differences in scope flags.
+  if (Cond && !getLangOpts().C99 && !getLangOpts().CPlusPlus &&
+      !Diags.isIgnored(diag::warn_comma_operator, Cond->getExprLoc()))
+    CommaVisitor(*this).Visit(Cond);
+
   DiagnoseUnusedExprResult(Body);
 
   return new (Context) DoStmt(Body, Cond, DoLoc, WhileLoc, CondRParen);