[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:7651-7652
+  Expr *FirstArg = TheCall->getArg(0);
+  if (auto *CE = dyn_cast<CastExpr>(FirstArg))
+    FirstArg = CE->getSubExprAsWritten();
 
----------------
rsmith wrote:
> This looks very suspicious to me: this will remove a cast expression that was written in the source code from the AST. That loses source fidelity, can give the wrong answer if the cast changed the value (such as a C++ derived-to-base conversion to a non-primary base class), and in any case this is only done once where there could be multiple explicit casts written on an argument to the builtin, so if it's necessary, then it's not being done fully.
> 
> Can this be removed?
Somehow I missed this in my review.  Yes, this line should be unnecessary, and as you say, it is definitely wrong.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133202/new/

https://reviews.llvm.org/D133202