On Mon, Aug 6, 2012 at 9:46 PM, Eli Friedman <[email protected]> wrote:
> On Mon, Aug 6, 2012 at 9:16 PM, Richard Smith > <[email protected]> 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. I agree. This is inherited from the old HasSideEffects implementation. I was intending to clean this up in a subsequent commit. > > + 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? > I've not found one, but we clearly should. This is duplicated now at least here, in Sema::BuildCXXTypeId and in CodeGenFunction::EmitCXXTypeidExpr. > > + 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? /// HasSideEffects - This routine returns true for all those expressions /// which must be evaluated each time and must not be optimized away /// or evaluated at compile time. Example is a function call, volatile /// variable read. ... although some further investigation indicates that we have some callers which want a stronger guarantee that evaluating the expression can't be detected. I'll remove the elidable check and update the comment.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
