[cfe-commits] r145990 - in /cfe/trunk: include/clang/AST/Expr.h lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp

[Ted Kremenek]

Interesting.  Thanks Richard.  I'll have to look into how we can better deal with this in DeadStoresChecker to correctly reflect that it is looking for just forms of defensive programming.

On Dec 6, 2011, at 5:26 PM, Richard Smith wrote:

> On Wed, December 7, 2011 00:31, Ted Kremenek wrote:
>> Is there a relevant test case?
> 
> Yes and no. Yes in that, there are rare cases[*] which we can emit as IR
> constants now, but can't evaluate, but also no, in the sense that there's no
> reason why we shouldn't be able to evaluate those expressions at some point in
> the future.
> 
> The purpose of this change was mostly to enforce layering for this function:
> it's now only used by CodeGen and by Sema to check whether CodeGen will be
> able to emit a constant (which is what it's for, and is not what the dead
> store checker was trying to check).
> 
> Ultimately, I suspect that using constant-folding is the wrong approach for
> this check, since I expect there'll be expressions which we can fold but which
> are 'obviously' not just defensive programming / -Wuninitialized suppression,
> but this patch did not try to fix that.
> 
> - Richard
> 
> [*] Here's such a testcase:
> 
> int f() {
>  int delta = &&b - &&a;
> a:
>  delta = 42;
> b:
>  return delta;
> }
> 
>> On Dec 6, 2011, at 3:25 PM, Richard Smith wrote:
>>> Author: rsmith
>>> Date: Tue Dec  6 17:25:15 2011
>>> New Revision: 145990
>>> 
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=145990&view=rev
>>> Log:
>>> DeadStoresChecker: when whitelisting dead initializations with constants,
>>> look for a foldable constant rather than an IR-level constant. This is still
>>> far too liberal, but is a step in the right direction.
>>> 
>>> Modified:
>>> cfe/trunk/include/clang/AST/Expr.h
>>> cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
>>> 
>>> Modified: cfe/trunk/include/clang/AST/Expr.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=
>>> 145990&r1=145989&r2=145990&view=diff
>>> ============================================================================
>>> ==
>>> --- cfe/trunk/include/clang/AST/Expr.h (original)
>>> +++ cfe/trunk/include/clang/AST/Expr.h Tue Dec  6 17:25:15 2011
>>> @@ -421,8 +421,9 @@
>>> llvm::APSInt X;
>>> return isIntegerConstantExpr(X, Ctx, Loc); }
>>> -  /// isConstantInitializer - Returns true if this expression is a constant
>>> -  /// initializer, which can be emitted at compile-time.
>>> +
>>> +  /// isConstantInitializer - Returns true if this expression can be
>>> emitted to +  /// IR as a constant, and thus can be used as a constant
>>> initializer in C. bool isConstantInitializer(ASTContext &Ctx, bool ForRef)
>>> const;
>>> 
>>> /// EvalStatus is a struct with detailed info about an evaluation in
>>> progress.
>>> 
>>> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/D
>>> eadStoresChecker.cpp?rev=145990&r1=145989&r2=145990&view=diff
>>> ===========================================================================
>>> ===
>>> --- cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp (original)
>>> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp Tue Dec  6
>>> 17:25:15 2011
>>> @@ -274,7 +274,7 @@
>>> // If x is EVER assigned a new value later, don't issue
>>> // a warning.  This is because such initialization can be
>>> // due to defensive programming.
>>> -              if (E->isConstantInitializer(Ctx, false))
>>> +              if (E->isEvaluatable(Ctx))
>>> return;
>>> 
>>> if (DeclRefExpr *DRE=dyn_cast<DeclRefExpr>(E->IgnoreParenCasts()))
>>> 
>>> 
>>> _______________________________________________
>>> cfe-commits mailing list cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>> 
>> 
>> 
>