[PATCH] D49508: [Sema] Mark implicitly-inserted ICE's as being part of explicit cast (PR38166)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked 3 inline comments as done.
lebedev.ri added a comment.

Thank you for the review!



================
Comment at: lib/Sema/SemaCast.cpp:94-101
+    void updatePartOfExplicitCastFlags(CastExpr *CE) {
+      // Walk down from the CE to the OrigSrcExpr, and mark all immediate
+      // ImplicitCastExpr's as being part of ExplicitCastExpr. The original CE
+      // (which is a ExplicitCastExpr), and the OrigSrcExpr are not touched.
+      while (OrigSrcExpr.get() != CE->getSubExpr() &&
+             (CE = dyn_cast<ImplicitCastExpr>(CE->getSubExpr())))
+        CE->setIsPartOfExplicitCast(true);
----------------
rsmith wrote:
> lebedev.ri wrote:
> > rsmith wrote:
> > > You don't need to track the `OrigSrcExpr` here. You can just recurse down through all the `ImplicitCastExpr`s (they're always all notionally part of the explicit cast).
> > We do sometimes have `OrigSrcExpr` being `ImplicitCastExpr`.
> > https://godbolt.org/g/S5951G <- that `ImplicitCastExpr` would now get marked, even though it is `OrigSrcExpr`.
> > Is that expected?
> Yes, that's expected, the conversion from `int` to `char` there is part of the semantics of the explicit cast from `int` to `vector of char`, and you wouldn't want your UBSan check to warn about that, right?
I was just surprized since that `ImplicitCastExpr` was there before this `CastExpr` handling.
If i make that splat implicit, it now warns about the truncation, https://godbolt.org/g/Ax7UTw
So yeah, i guess this is correct.


Repository:
  rC Clang

https://reviews.llvm.org/D49508