vsk added a comment.
Thanks for working on this!
================
Comment at: docs/ReleaseNotes.rst:277
+ behaviour. But they are not *always* intentional, and are somewhat hard to
+ track down.
----------------
Could you mention whether the group is enabled by -fsanitize=undefined?
================
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);
----------------
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.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:951
+// implicit cast -> explicit cast <- that is an explicit
cast.
+static bool CastIsPartOfExplictCast(ASTContext &Ctx, const Expr *E) {
+ // If it's a nulllptr, or not a cast, then it's not a part of Explict Cast.
----------------
nit, function names typically begin with a verb: 'isCastPartOf...'
================
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();
----------------
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.
Repository:
rC Clang
https://reviews.llvm.org/D48958
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
