[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thank you!! I think we're almost ready to commit but this concern is still hanging:

> I see that the patch doesn't touch `handleFixableVariable()`. Do we still attach fixits to the warning, or did we already change it to be attached to a note associated with the warning? Because fixits with placeholders aren't allowed on warnings, we should make sure it's attached to note before landing this patch.



================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:780-781
+  } else {
+    // In cases `Init` is of the form `&Var` after stripping of implicit
+    // casts, where `&` is the built-in operator, the extent is 1.
+    if (auto AddrOfExpr = dyn_cast<UnaryOperator>(Init->IgnoreImpCasts()))
----------------
ziqingluo-90 wrote:
> NoQ wrote:
> > ```lang=c
> > int x = 1;
> > char *ptr = &x; // std::span<char> ptr { &x, 4 };
> > ```
> > This is valid code. I suspect we want to check types as well, to see that type sizes match.
> > 
> > Most of the time code like this violates strict aliasing, but `char` is exceptional, and even if it did violate strict aliasing, people can compile with `-fno-strict-aliasing` to define away the UB, so we have to respect that.
> This code is not valid in C++.  An explicit cast is needed in front of `&x`.  I will add a test to show that 
> 
> ```
> int x = 1;
> char * ptr = (char *)&x;
> ```
> will have a place holder for the span size.
Yes you're right! It's only valid in C where these fixits don't apply.


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

https://reviews.llvm.org/D139737