flx created this revision.
flx added reviewers: aaron.ballman, ymandel, gribozavr2.
Herald added a subscriber: xazax.hun.
flx requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Structured bindings can currently trigger the check and lead to a wrong
fix. Because the DecompositionDecl itself is not used and the check does not
iterate through its the decl's bindings to verify whether the bindings' holding
vars are used this leads to the whole statement to be deleted.
To support structured bindings properly 3 cases would need to be considered.
1. All holding vars are not used -> The statement can be deleted.
2. All holding vars are used as const or not used -> auto can be converted to
const auto&.
3. Neither case is true -> leave unchanged.
In the check we'll have to separate the logic that determines this from the code
that produces the diagnostic and fixes and first determine which of the cases
we're dealing with before creating fixes.
Since this is a bigger refactoring we'll disable structured bindings for now to
prevent incorrect fixes.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D105727
Files:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===================================================================
---
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
+// RUN: %check_clang_tidy -std=c++17 %s
performance-unnecessary-copy-initialization %t
template <typename T>
struct Iterator {
@@ -637,3 +637,18 @@
}
};
}
+
+void negativeStructuredBinding() {
+ // Structured bindings are not yet supported but can trigger false positives
+ // since the DecompositionDecl itself is unused and the check doesn't
traverse
+ // VarDecls of the BindingDecls.
+ struct Pair {
+ ExpensiveToCopyType first;
+ ExpensiveToCopyType second;
+ };
+
+ Pair P;
+ const auto [C, D] = P;
+ C.constMethod();
+ D.constMethod();
+}
Index:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -147,6 +147,7 @@
return compoundStmt(
forEachDescendant(
declStmt(
+ unless(has(decompositionDecl())),
has(varDecl(hasLocalStorage(),
hasType(qualType(
hasCanonicalType(allOf(
Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
+// RUN: %check_clang_tidy -std=c++17 %s performance-unnecessary-copy-initialization %t
template <typename T>
struct Iterator {
@@ -637,3 +637,18 @@
}
};
}
+
+void negativeStructuredBinding() {
+ // Structured bindings are not yet supported but can trigger false positives
+ // since the DecompositionDecl itself is unused and the check doesn't traverse
+ // VarDecls of the BindingDecls.
+ struct Pair {
+ ExpensiveToCopyType first;
+ ExpensiveToCopyType second;
+ };
+
+ Pair P;
+ const auto [C, D] = P;
+ C.constMethod();
+ D.constMethod();
+}
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -147,6 +147,7 @@
return compoundStmt(
forEachDescendant(
declStmt(
+ unless(has(decompositionDecl())),
has(varDecl(hasLocalStorage(),
hasType(qualType(
hasCanonicalType(allOf(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
