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
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
===================================================================
--- clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
@@ -56,12 +56,46 @@
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:24}:"std::span<int const> const q"
// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:25-[[@LINE-2]]:25}:"{"
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:26-[[@LINE-3]]:26}:", 10}"
+ [[deprecated]] const int * x = a;
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:18-[[@LINE-1]]:33}:"std::span<int const> x"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:34-[[@LINE-2]]:34}:"{"
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:35-[[@LINE-3]]:35}:", 10}"
+ const int * y [[deprecated]];
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:16}:"std::span<int const> y"
+
int tmp;
+
tmp = p[5];
tmp = q[5];
+ tmp = x[5];
+ tmp = y[5];
}
+void local_variable_unsupported_specifiers() {
+ int a[10];
+ const int * p [[deprecated]] = a; // not supported because the attribute overlaps the source range of the declaration
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+
+ static const int * q = a; // storage specifier not supported yet
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+
+ extern int * x; // storage specifier not supported yet
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+
+ constexpr int * y = 0; // `constexpr` specifier not supported yet
+ // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:
+
+ int tmp;
+
+ tmp = p[5];
+ tmp = q[5];
+ tmp = x[5];
+ tmp = y[5];
+}
+
+
+
void local_array_subscript_variable_extent() {
int n = 10;
int tmp;
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===================================================================
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1381,6 +1381,26 @@
return getRangeText({ParmIdentBeginLoc, ParmIdentEndLoc}, SM, LangOpts);
}
+// We cannot fix a variable declaration if it has some other specifiers than the
+// type specifier. Because the source ranges of those specifiers could overlap
+// with the source range that is being replaced using fix-its. Especially when
+// we often cannot obtain accruate source ranges of cv-qualified type
+// specifiers.
+static bool hasUnsupportedSpecifiers(const VarDecl *VD,
+ const SourceManager &SM) {
+ // AttrRangeOverlapping: true if at least one attribute of `VD` overlaps the
+ // source range of `VD`:
+ bool AttrRangeOverlapping = llvm::any_of(VD->attrs(), [&](Attr *At) -> bool {
+ return !(SM.isBeforeInTranslationUnit(At->getRange().getEnd(),
+ VD->getBeginLoc())) &&
+ !(SM.isBeforeInTranslationUnit(VD->getEndLoc(),
+ At->getRange().getBegin()));
+ });
+ return VD->isInlineSpecified() || VD->isConstexpr() ||
+ VD->hasConstantInitialization() || !VD->hasLocalStorage() ||
+ AttrRangeOverlapping;
+}
+
// Returns the text of the pointee type of `T` from a `VarDecl` of a pointer
// type. The text is obtained through from `TypeLoc`s. Since `TypeLoc` does not
// have source ranges of qualifiers ( The `QualifiedTypeLoc` looks hacky too me
@@ -1801,6 +1821,9 @@
// list otherwise.
static FixItList fixLocalVarDeclWithSpan(const VarDecl *D, ASTContext &Ctx,
const StringRef UserFillPlaceHolder) {
+ if (hasUnsupportedSpecifiers(D, Ctx.getSourceManager()))
+ return {};
+
FixItList FixIts{};
std::optional<std::string> SpanTyText = createSpanTypeForVarDecl(D, Ctx);
@@ -2028,6 +2051,8 @@
// `createOverloadsForFixedParams`).
static FixItList fixParamWithSpan(const ParmVarDecl *PVD, const ASTContext &Ctx,
UnsafeBufferUsageHandler &Handler) {
+ if (hasUnsupportedSpecifiers(PVD, Ctx.getSourceManager()))
+ return {};
if (PVD->hasDefaultArg())
// FIXME: generate fix-its for default values:
return {};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
