lebedev.ri added inline comments.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:318
- Value *EmitScalarConversion(Value *Src, QualType SrcTy, QualType DstTy,
+ Value *EmitScalarConversion(Value *Src, QualType SrcType, QualType DstType,
SourceLocation Loc, bool TreatBooleanAsSigned);
----------------
vsk wrote:
> I think the number of overloads here is really unwieldy. There should be a
> simpler way to structure this. What about consolidating all four overloads
> into one? Maybe:
>
> ```
> struct ScalarConversionsOpts {
> bool TreatBoolAsUnsigned = false;
> bool EmitImplicitIntegerTruncationCheck = false;
> };
>
> Value *EmitScalarConversion(Src, SrcTy, DstTy, Loc, Opts =
> ScalarConversionOpts())
> ```
>
> It's not necessary to pass CastExpr in, right? There's only one place where
> that's done. It seems simpler to just do the SanOpts /
> isCastPartOfExplicitCast checking there.
The number of overloads is indeed unwieldy.
================
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:
> 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..
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
