patch for warning on logical negation with known result

[jahanian]

In r222009 with minor caveat. I was forced to make Sema::CheckBoolLikeConversion
public because it is called from static function.

- Fariborz


On Nov 13, 2014, at 4:16 PM, Richard Trieu <rtrieu at google.com> wrote:

> Looks good, just a few small changes.  Good to commit after changes are made.
>  
> Index: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h	(revision 221791)
> +++ include/clang/Sema/Sema.h	(working copy)
> @@ -2982,6 +2982,7 @@
>      return TypoCorrection();
>    }
>  
> +  void CheckBoolLikeConversion(Expr *E, SourceLocation CC);
> Move this function declaration next to the declaration for CheckImplicitConversions 
>  public:
>    /// AddInstanceMethodToGlobalPool - All instance methods in a translation
>    /// unit are added to a global pool. This allows us to efficiently associate
> Index: lib/Sema/SemaChecking.cpp
> ===================================================================
> --- lib/Sema/SemaChecking.cpp	(revision 221791)
> +++ lib/Sema/SemaChecking.cpp	(working copy)
> @@ -6480,6 +6480,13 @@
>                              E->getType(), CC, &Suspicious);
>  }
>  
> Add comment to what this function is checking. 
> +static void CheckBoolLikeConversion(Sema &S, Expr *E, SourceLocation CC) {
> +  if (S.getLangOpts().Bool)
> +    return;
> +  CheckImplicitConversion(S, E->IgnoreParenImpCasts(),
> +                          S.Context.BoolTy, CC);
> This should all fit on one line. 
>  
> +}
> +
>  /// AnalyzeImplicitConversions - Find and report any interesting
>  /// implicit conversions in the given expression.  There are a couple
>  /// of competing diagnostics here, -Wconversion and -Wsign-compare.
> @@ -6532,6 +6539,11 @@
>      // And with simple assignments.
>      if (BO->getOpcode() == BO_Assign)
>        return AnalyzeAssignment(S, BO);
> +      
> +    if (BO->isLogicalOp()) {
> +      CheckBoolLikeConversion(S, BO->getLHS(), BO->getLHS()->getExprLoc());
> +      CheckBoolLikeConversion(S, BO->getRHS(), BO->getRHS()->getExprLoc());
> +    }
> Move this down next to the UnaryOperator check.  BO exists down there, so "if (BO && BO->isLogicalOp())" will work. 
>    }
>  
>    // These break the otherwise-useful invariant below.  Fortunately,
> @@ -6559,6 +6571,9 @@
>        continue;
>      AnalyzeImplicitConversions(S, ChildExpr, CC);
>    }
> +  if (const UnaryOperator *U = dyn_cast<UnaryOperator>(E))
> +    if (U->getOpcode() == UO_LNot)
> +        CheckBoolLikeConversion(S, U->getSubExpr(), CC);
>  }
>  
>  } // end anonymous namespace
> @@ -6617,6 +6632,10 @@
>    return false;
>  }
>  
> +void Sema::CheckBoolLikeConversion(Expr *E, SourceLocation CC) {
> +  ::CheckBoolLikeConversion(*this, E, CC);
> +}
> Move this function next to the other CheckBoolLikeConversion 
> +
>  /// \brief Diagnose pointers that are always non-null.
>  /// \param E the expression containing the pointer
>  /// \param NullKind NPCK_NotNull if E is a cast to bool, otherwise, E is
> Index: lib/Sema/SemaExpr.cpp
> ===================================================================
> --- lib/Sema/SemaExpr.cpp	(revision 221791)
> +++ lib/Sema/SemaExpr.cpp	(working copy)
> @@ -8413,7 +8413,7 @@
>      if (!LHS.get()->getType()->isScalarType() ||
>          !RHS.get()->getType()->isScalarType())
>        return InvalidOperands(Loc, LHS, RHS);
> -
> +    
> Remove whitespace change. 
>      return Context.IntTy;
>    }
>  
> @@ -12982,6 +12982,7 @@
>          << T << E->getSourceRange();
>        return ExprError();
>      }
> +    CheckBoolLikeConversion(E, Loc);
>    }
>  
>    return E;
> 
> On Thu, Nov 13, 2014 at 8:12 AM, jahanian <fjahanian at apple.com> wrote:
> - PING.
> Thanks, Fariborz
> 
> On Nov 12, 2014, at 9:11 AM, jahanian <fjahanian at apple.com> wrote:
> 
> >
> > Revised patch per Richard T’s comments. Please review.
> > - Fariborz
> >
> > <patch.txt>_______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141114/5617556f/attachment.html>