patch for warning on logical negation with known result

Richard Trieu rtrieu at google.com
Thu Nov 13 16:16:37 PST 2014


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/20141113/14859f2f/attachment.html>


More information about the cfe-commits mailing list