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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
