vsk added a comment.
I have some minor comments but overall I think this is in good shape. It would
be great to see some compile-time numbers just to make sure this is tractable.
I'm pretty sure -fsanitize=null would fire more often across a codebase than
this check, so I don't anticipate a big surprise here.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:222
+
+ // The stack of currently-visiting Cast expressions.
+ llvm::SmallVector<CastExpr *, 8> CastExprStack;
----------------
It would help to have this comment explain that the stack is used/maintained
exclusively by the implicit cast sanitizer.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:228
+
+ // We only care if we are to use this data.
+ bool Enabled() {
----------------
Could you make this comment more specific -- maybe by explaining that for
efficiency reasons, the cast expr stack is only maintained when a sanitizer
check is enabled?
================
Comment at: lib/CodeGen/CGExprScalar.cpp:234
+ public:
+ CastExprStackGuard(ScalarExprEmitter *SEE, CastExpr *CE) : SEE(SEE) {
+ if (!Enabled())
----------------
I think if you were to use references instead of pointers here, the code would
be a bit clearer, and you wouldn't need to assert that CE is non-null.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:351
+ ScalarConversionOpts()
+ : TreatBooleanAsSigned(false),
+ EmitImplicitIntegerTruncationChecks(false) {}
----------------
Why not use default member initializers here (e.g, "bool a = false")?
================
Comment at: lib/CodeGen/CGExprScalar.cpp:988
+ return Child == PreviousCast;
+ }))
+ return false;
----------------
The none_of call could safely be replaced by `Cast->getSubExpr() !=
PreviousCast`, I think.
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
