gnanabit created this revision.
gnanabit added a reviewer: courbet.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
gnanabit requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
In the following code:
cc
struct Obj {
Obj();
Obj(const Obj &);
Obj(Obj &&);
virtual ~Obj();
};
Obj ConstNrvo() {
const Obj obj;
return obj;
}
performance-no-automatic-move warns about the constness of `obj`. However, NRVO
is applied to `obj`, so the `const` should have no effect on performance.
This change modifies the matcher to exclude NRVO variables.
#clang-tidy <https://reviews.llvm.org/tag/clang-tools-extra/>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147419
Files:
clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
@@ -33,10 +33,15 @@
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents
automatic move [performance-no-automatic-move]
}
-Obj PositiveSelfConstValue() {
- const Obj obj = Make<Obj>();
- return obj;
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents
automatic move [performance-no-automatic-move]
+Obj PositiveCantNrvo(bool b) {
+ const Obj obj1;
+ const Obj obj2;
+ if (b) {
+ return obj1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: constness of 'obj1' prevents
automatic move [performance-no-automatic-move]
+ }
+ return obj2;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj1' prevents
automatic move [performance-no-automatic-move]
}
// FIXME: Ideally we would warn here too.
@@ -54,18 +59,32 @@
// Negatives.
StatusOr<Obj> Temporary() {
- return Make<const Obj>();
+ return Make<Obj>();
}
StatusOr<Obj> ConstTemporary() {
return Make<const Obj>();
}
-StatusOr<Obj> Nrvo() {
+StatusOr<Obj> ConvertingMoveConstructor() {
Obj obj = Make<Obj>();
return obj;
}
+Obj ConstNrvo() {
+ const Obj obj = Make<Obj>();
+ return obj;
+}
+
+Obj NotNrvo(bool b) {
+ Obj obj1;
+ Obj obj2;
+ if (b) {
+ return obj1;
+ }
+ return obj2;
+}
+
StatusOr<Obj> Ref() {
Obj &obj = Make<Obj &>();
return obj;
Index: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
@@ -16,6 +16,12 @@
namespace clang::tidy::performance {
+namespace {
+
+AST_MATCHER(VarDecl, isNRVOVariable) { return Node.isNRVOVariable(); }
+
+} // namespace
+
NoAutomaticMoveCheck::NoAutomaticMoveCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@@ -23,8 +29,9 @@
utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
void NoAutomaticMoveCheck::registerMatchers(MatchFinder *Finder) {
- const auto ConstLocalVariable =
+ const auto NonNrvoConstLocalVariable =
varDecl(hasLocalStorage(), unless(hasType(lValueReferenceType())),
+ unless(isNRVOVariable()),
hasType(qualType(
isConstQualified(),
hasCanonicalType(matchers::isExpensiveToCopy()),
@@ -48,7 +55,7 @@
cxxConstructExpr(
hasDeclaration(LValueRefCtor),
hasArgument(0, ignoringParenImpCasts(declRefExpr(
- to(ConstLocalVariable)))))
+ to(NonNrvoConstLocalVariable)))))
.bind("ctor_call")))))),
this);
}
Index: clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
@@ -33,10 +33,15 @@
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
}
-Obj PositiveSelfConstValue() {
- const Obj obj = Make<Obj>();
- return obj;
- // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+Obj PositiveCantNrvo(bool b) {
+ const Obj obj1;
+ const Obj obj2;
+ if (b) {
+ return obj1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: constness of 'obj1' prevents automatic move [performance-no-automatic-move]
+ }
+ return obj2;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj1' prevents automatic move [performance-no-automatic-move]
}
// FIXME: Ideally we would warn here too.
@@ -54,18 +59,32 @@
// Negatives.
StatusOr<Obj> Temporary() {
- return Make<const Obj>();
+ return Make<Obj>();
}
StatusOr<Obj> ConstTemporary() {
return Make<const Obj>();
}
-StatusOr<Obj> Nrvo() {
+StatusOr<Obj> ConvertingMoveConstructor() {
Obj obj = Make<Obj>();
return obj;
}
+Obj ConstNrvo() {
+ const Obj obj = Make<Obj>();
+ return obj;
+}
+
+Obj NotNrvo(bool b) {
+ Obj obj1;
+ Obj obj2;
+ if (b) {
+ return obj1;
+ }
+ return obj2;
+}
+
StatusOr<Obj> Ref() {
Obj &obj = Make<Obj &>();
return obj;
Index: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
@@ -16,6 +16,12 @@
namespace clang::tidy::performance {
+namespace {
+
+AST_MATCHER(VarDecl, isNRVOVariable) { return Node.isNRVOVariable(); }
+
+} // namespace
+
NoAutomaticMoveCheck::NoAutomaticMoveCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@@ -23,8 +29,9 @@
utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
void NoAutomaticMoveCheck::registerMatchers(MatchFinder *Finder) {
- const auto ConstLocalVariable =
+ const auto NonNrvoConstLocalVariable =
varDecl(hasLocalStorage(), unless(hasType(lValueReferenceType())),
+ unless(isNRVOVariable()),
hasType(qualType(
isConstQualified(),
hasCanonicalType(matchers::isExpensiveToCopy()),
@@ -48,7 +55,7 @@
cxxConstructExpr(
hasDeclaration(LValueRefCtor),
hasArgument(0, ignoringParenImpCasts(declRefExpr(
- to(ConstLocalVariable)))))
+ to(NonNrvoConstLocalVariable)))))
.bind("ctor_call")))))),
this);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
