[cfe-commits] r161388 - in /cfe/trunk: lib/AST/Expr.cpp lib/AST/ExprConstant.cpp test/CodeGenCXX/const-init-cxx11.cpp test/CodeGenCXX/debug-lambda-expressions.cpp test/CodeGenCXX/global-init.cpp test/CodeGenCXX/lambda-expressions.cpp test/CodeGen

[Eli Friedman]

On Mon, Aug 6, 2012 at 9:16 PM, Richard Smith
<richard-llvm at metafoo.co.uk> wrote:
> Author: rsmith
> Date: Mon Aug  6 23:16:51 2012
> New Revision: 161388
>
> URL: http://llvm.org/viewvc/llvm-project?rev=161388&view=rev
> Log:
> Teach Expr::HasSideEffects about all the Expr types, and fix a bug where it
> was mistakenly classifying dynamic_casts which might throw as having no side
> effects.
>
> Switch it from a visitor to a switch, so it is kept up-to-date as future Expr
> nodes are added. Move it from ExprConstant.cpp to Expr.cpp, since it's not
> really related to constant expression evaluation.
>
> Since we use HasSideEffect to determine whether to emit an unused global with
> internal linkage, this has the effect of suppressing emission of globals in
> some cases.
>
> I've left many of the Objective-C cases conservatively assuming that the
> expression has side-effects. I'll leave it to someone with better knowledge
> of Objective-C than mine to improve them.
>
> +  case DeclRefExprClass:
> +  case ObjCIvarRefExprClass:
> +    return getType().isVolatileQualified();

Saying that a DeclRefExpr has a side-effect if it's volatile is weird
and incomplete... an lvalue-to-rvalue conversion can have a
side-effect, but the DeclRef itself doesn't.  "&x" for any normal
variable x doesn't have a side-effect whether or not the type is
volatile-qualified, and "*(volatile int*)0x1234" has a side-effect
even though I think the current HasSideEffects will conclude it
doesn't.

> +  case CXXTypeidExprClass: {
> +    // A typeid expression has side-effects if it can throw.
> +    const CXXTypeidExpr *TE = cast<CXXTypeidExpr>(this);
> +    if (TE->isTypeOperand())
> +      return false;
> +    const CXXRecordDecl *RD =
> +        TE->getExprOperand()->getType()->getAsCXXRecordDecl();
> +    if (!RD || !RD->isPolymorphic() ||
> +        !TE->getExprOperand()->
> +          Classify(const_cast<ASTContext&>(Ctx)).isGLValue())
> +      // Not a glvalue of polymorphic class type: the expression is an
> +      // unevaluated operand.
> +      return false;
> +    // Might throw.
> +    return true;
> +  }

Do we really not have a helper routine for this?

> +  case CXXConstructExprClass:
> +  case CXXTemporaryObjectExprClass: {
> +    const CXXConstructExpr *CE = cast<CXXConstructExpr>(this);
> +    if (!CE->isElidable() && !CE->getConstructor()->isTrivial())
> +      return true;
> +    // An elidable or trivial constructor does not add any side-effects of its
> +    // own. Just look at its arguments.
> +    break;
> +  }

It's not obvious to me that an elidable constructor doesn't add
side-effects; what exactly is HasSideEffects defined to return?

-Eli