[cfe-commits] r172016 - in /cfe/trunk: include/clang/AST/Expr.h lib/AST/ExprConstant.cpp lib/Sema/SemaStmt.cpp test/Sema/switch-1.c

[jahanian]

On Jan 23, 2013, at 8:28 PM, Richard Smith <richard at metafoo.co.uk> wrote:

>> 
>> This patch should be feature complete.
> 
> +  void EvaluateForOverflow(const ASTContext &Ctx,
> +                            SmallVectorImpl<PartialDiagnosticAt> *Diag) const;
> 
> Extra space on second line.
> 
> +      // Sould return true in IntOverflowCheckMode, so that we check
> for overflow
> +      // even if some subexpressions can't be evaluated as constants.
> +      // subexpressions can't be evaluated as constants.
> 
> Typo 'Sould' and extra line.
> 
> Other than that, this looks fine, assuming there's no performance hit.
> 


Typos fixed and patch checked in r173377. I also, compiled couple of the
largest Sema files and timed them using our internal measurement tool and did not 
noticed any noticeable change. In the course of compiling these files,  I noticed a 
crash in the expression evaluator and fixed it using this additional change (which now
successfully builds llvm).

-void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc) {
+void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc,
+                              bool IsConstexpr) {
  CheckImplicitConversions(E, CheckLoc);
  CheckUnsequencedOperations(E);
+  if (!IsConstexpr && !E->isValueDependent())
+    CheckForIntOverflow(E);
}

Note the addition of check on expression's value dependency. I was leery to
add this check deep inside the evaluator as I am not comfortable doing that.

- Thanks, Fariborz