This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
ziqingluo-90 marked an inline comment as done.
Closed by commit rGb58e52889808: [-Wunsafe-buffer-usage] Stop generating
incorrect fix-its for variable… (authored by ziqingluo-90).
Changed prior to commit:
https://reviews.llvm.org/D156192?vs=543760&id=552162#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156192/new/
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
@@ -58,12 +58,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
@@ -1416,6 +1416,27 @@
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 accurate source ranges of cv-qualified type
+// specifiers.
+// FIXME: also deal with type attributes
+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
@@ -1841,8 +1862,11 @@
// the non-empty fix-it list, if fix-its are successfuly generated; empty
// list otherwise.
static FixItList fixLocalVarDeclWithSpan(const VarDecl *D, ASTContext &Ctx,
- const StringRef UserFillPlaceHolder,
- UnsafeBufferUsageHandler &Handler) {
+ const StringRef UserFillPlaceHolder,
+ UnsafeBufferUsageHandler &Handler) {
+ if (hasUnsupportedSpecifiers(D, Ctx.getSourceManager()))
+ return {};
+
FixItList FixIts{};
std::optional<std::string> SpanTyText = createSpanTypeForVarDecl(D, Ctx);
@@ -2076,6 +2100,10 @@
// `createOverloadsForFixedParams`).
static FixItList fixParamWithSpan(const ParmVarDecl *PVD, const ASTContext &Ctx,
UnsafeBufferUsageHandler &Handler) {
+ if (hasUnsupportedSpecifiers(PVD, Ctx.getSourceManager())) {
+ DEBUG_NOTE_DECL_FAIL(PVD, " : has unsupport specifier(s)");
+ return {};
+ }
if (PVD->hasDefaultArg()) {
// FIXME: generate fix-its for default values:
DEBUG_NOTE_DECL_FAIL(PVD, " : has default arg");
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits