lebedev.ri added inline comments.
================ Comment at: lib/CodeGen/CGExprScalar.cpp:1694 // handle things like function to ptr-to-function decay etc. Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) { Expr *E = CE->getSubExpr(); ---------------- vsk wrote: > lebedev.ri wrote: > > vsk wrote: > > > I think maintaining a stack of visited cast exprs in the emitter be > > > cheaper/simpler than using ASTContext::getParents. You could push CE here > > > and use a RAII helper to pop it. The 'isCastPartOfExplicitCast' check > > > then simplifies to a quick stack traversal. > > Hmm, two things come to mind: > > 1. This pessimizes the (most popular) case when the sanitizer is disabled. > > 2. `ASTContext::getParents()` may return more than one parent. I'm not sure > > if that matters here? > > I'll take a look.. > As for (1), it's not necessary to push/pop the stack when this sanitizer is > disabled. And for (2), IIUC the only interesting case is "explicit-cast > <implicit-cast>+", and none of the implicit casts here have more than one > parent. > I think maintaining a stack of visited cast exprs in the emitter be > cheaper/simpler than using ASTContext::getParents. {F6660377} So yeah, this could work. Except sadly it breaks in subtle cases like https://godbolt.org/g/5V2czU I have added those tests beforehand. Is `ASTContext::getParents()` really horribly slow so we want to duplicate/maintain/track the current AST stack ourselves? If so, we will need to maintain the *entire* stack, not just `CastExpr''s... Repository: rC Clang https://reviews.llvm.org/D48958 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits