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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:385
+
+class Strategy;
+
----------------
There's already a forward declaration on line 144!


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:412
+        arraySubscriptExpr(BaseIsArrayOrPtrDRE,
+                           unless(hasIndex(integerLiteral(equals(0)))))
+            .bind(ULCArraySubscriptTag);
----------------
Subscripts `[0]` might be safe, but this doesn't mean we should avoid emitting fixits when we see them. I think this clause should be dropped here. (Might be worth adding a test).


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:723
+  OS << "{";
+  Init->printPretty(OS, nullptr, PP);
+  OS << ", ";
----------------
Uhh, I'm worried about this approach. I'm not sure these pretty-printers are even correct. They try to look similar to the underlying code, but I don't think they're required to. And even if they were, It only takes one forgotten override in `StmtPrinter` to produce incorrect pretty-prints that won't compile. So it's very unreliable technology.

I think the intended way to do these things is to simply avoid overwriting code that you want to preserve. Instead, modify code around it. Eg., if you want to change
```lang=c++
int *x = foo();
```
to
```lang=c++
std::span<int> x { foo(), N };
```
then you add `std::span<` before `int`, then don't touch `int`, then replace ` *` with `> `, then replace `=` with ` {`, then don't touch `foo()`, then add `, N }` immediately after.

If you actually need to move code around (eg., make preserved chunks appear in a different order), I think this should go through a facility like `Lexer::getSourceText()` which would give you the exact source code as written.


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

https://reviews.llvm.org/D139737