[PATCH] D156192: [-Wunsafe-buffer-usage] Stop generating incorrect fix-its for variable declarations with unsupported specifiers

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 created this revision.
ziqingluo-90 added reviewers: NoQ, jkorous, t-rasmud, malavikasamak.
Herald added a project: All.
ziqingluo-90 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We have to give up on fixing a variable declaration if it has specifiers that are not supported yet.  We cannot support them for now due to the combination of following challenges:

- Sometimes we do not have accurate source range information for the type specifiers of a variable declaration,  especially when cv-qualifiers are used (source location is not provided for type qualifiers <https://github.com/llvm/llvm-project/blob/5fa5c39871c8ea7df2173951be3a4a17b29fa42e/clang/include/clang/AST/TypeLoc.h#L280C15-L280C15>).   So it is hard to generate fix-its that only touch type specifiers for a declaration.
- We do not have the source range information for most declaration specifiers, such as storage specifiers.  So it is hard to tell whether a fix-it may quietly remove a specifier by accidentally overwriting it.
- When the declarator is not a trivial identifier (e.g., `int a[]` as a parameter decl),  we have to replace the whole declaration in order to fix it (e.g., to `std::span<int> a`).  If there are other specifiers involved, they may be accidentally removed (e.g.,  fix `int [[some_type_attribute]] a[]` to `std::span<int> a` is incorrect).

We could support these specifiers incrementally using the same approach as how we deal with cv-qualifiers.  If a fixing variable declaration has a storage specifier, instead of trying to find out the source location of the specifier or to avoid touching it, we add the keyword to a canonicalized place in the fix-it text that replaces the whole declaration.

For example, storage specifiers could be always placed at the beginning of a declaration. So both `const static int * x` and `static const int * x` will be fixed to `static std::span<const int> x`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156192

Files:
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D156192.543760.patch
Type: text/x-patch
Size: 4136 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230725/c1187e58/attachment-0001.bin>