rtrieu marked 2 inline comments as done.
rtrieu added inline comments.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8161
+ "bitwise or with non-zero value always evaluates to true">,
+ InGroup<TautologicalBitwiseCompare>, DefaultIgnore;
def warn_tautological_overlap_comparison : Warning<
----------------
jfb wrote:
> rtrieu wrote:
> > jfb wrote:
> > > Why default ignore?
> > This warning, like the tautological overlap warning, uses the CFG.
> > CFG-based analysis are typically excluded from being default warnings due
> > to the extra work of construction the CFG.
> Can you say so in the commit message?
Updated patch description.
================
Comment at: lib/Analysis/CFG.cpp:1118
+ Expr::EvalResult Result;
+ if (!Constant->EvaluateAsInt(Result, *Context))
+ return {};
----------------
NoQ wrote:
> It's kinda strange to me that we first confirm that it's a constant by doing
> `tryTransformToIntOrEnumConstant`, but then fire up the heavy machinery of
> `EvaluateAsInt` anyway. Did you consider using only `EvaluateAsInt()` to
> begin with? I guess you're trying to make sure that "the user's intent is
> clear" as other similar warnings do, right? Could you comment on that?
A new function has been created to check the zero-ness of the Expr. Since we
know it the Expr comes from tryTransformToIntOrEnumConstant, this function
doesn't have to handle the full Expr sub-classes.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66046/new/
https://reviews.llvm.org/D66046
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
