This revision was automatically updated to reflect the committed changes.
Closed by commit rL299638: [clang-tidy] Check for forwarding reference overload
in constructors. (authored by xazax).
Changed prior to commit:
https://reviews.llvm.org/D30547?vs=94028=94339#toc
Repository:
rL LLVM
aaron.ballman added a comment.
LGTM! If you need someone to commit on your behalf, let us know. I won't be
able to do it this week. but I'm guessing @alexfh or someone else may be
available to help (or I can get to it next week).
Repository:
rL LLVM
https://reviews.llvm.org/D30547
leanil updated this revision to Diff 94028.
leanil added a comment.
fix new lines
Repository:
rL LLVM
https://reviews.llvm.org/D30547
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
clang-tidy/misc/ForwardingReferenceOverloadCheck.h
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
Aside from some minor nits about newlines, LGTM!
Comment at: docs/clang-tidy/checks/misc-forwarding-reference-overload.rst:51-52
+
+
+
You can
leanil updated this revision to Diff 93774.
leanil added a comment.
Simplify checking the presence of copy and move ctors.
Repository:
rL LLVM
https://reviews.llvm.org/D30547
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:119
+ DisabledMove = false;
+ for (const auto *OtherCtor : Ctor->getParent()->ctors()) {
+if (OtherCtor->isCopyConstructor()) {
leanil wrote:
> This
leanil marked 2 inline comments as done.
leanil added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:119
+ DisabledMove = false;
+ for (const auto *OtherCtor : Ctor->getParent()->ctors()) {
+if (OtherCtor->isCopyConstructor()) {
leanil updated this revision to Diff 93114.
leanil added a comment.
Correct earlier diff issue with outdated master.
Repository:
rL LLVM
https://reviews.llvm.org/D30547
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
JonasToth added a comment.
Why does diff contain so many files?
Could you maybe merge the latest master into your branch?
Repository:
rL LLVM
https://reviews.llvm.org/D30547
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126
+}
+diag(Ctor->getLocation(), "function %0 can hide copy and move
constructors")
+<< Ctor;
+ }
leanil wrote:
> aaron.ballman wrote:
>
leanil marked 3 inline comments as done.
leanil added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126
+}
+diag(Ctor->getLocation(), "function %0 can hide copy and move
constructors")
+<< Ctor;
+ }
leanil updated this revision to Diff 92765.
leanil added a comment.
Simplify the note generation and correct the documentation.
Repository:
rL LLVM
https://reviews.llvm.org/D30547
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126
+}
+diag(Ctor->getLocation(), "function %0 can hide copy and move
constructors")
+<< Ctor;
+ }
xazax.hun wrote:
> aaron.ballman wrote:
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126
+}
+diag(Ctor->getLocation(), "function %0 can hide copy and move
constructors")
+<< Ctor;
+ }
xazax.hun wrote:
> aaron.ballman wrote:
xazax.hun added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126
+}
+diag(Ctor->getLocation(), "function %0 can hide copy and move
constructors")
+<< Ctor;
+ }
aaron.ballman wrote:
> xazax.hun wrote:
> >
xazax.hun added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126
+}
+diag(Ctor->getLocation(), "function %0 can hide copy and move
constructors")
+<< Ctor;
+ }
aaron.ballman wrote:
> aaron.ballman wrote:
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:125-126
+}
+diag(Ctor->getLocation(), "function %0 can hide copy and move
constructors")
+<< Ctor;
+ }
aaron.ballman wrote:
> leanil wrote:
>
leanil updated this revision to Diff 92542.
leanil added a comment.
Suppress warning only on std::enable_if.
Make note on copy and move constructors.
Repository:
rL LLVM
https://reviews.llvm.org/D30547
Files:
clang-tidy/misc/CMakeLists.txt
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/MiscTidyModule.cpp:70
+CheckFactories.registerCheck(
+"misc-forwarding-reference-overload");
CheckFactories.registerCheck("misc-misplaced-const");
xazax.hun wrote:
>
xazax.hun added inline comments.
Comment at: clang-tidy/misc/MiscTidyModule.cpp:70
+CheckFactories.registerCheck(
+"misc-forwarding-reference-overload");
CheckFactories.registerCheck("misc-misplaced-const");
malcolm.parsons wrote:
>
malcolm.parsons added inline comments.
Comment at: clang-tidy/misc/MiscTidyModule.cpp:70
+CheckFactories.registerCheck(
+"misc-forwarding-reference-overload");
CheckFactories.registerCheck("misc-misplaced-const");
aaron.ballman wrote:
> leanil
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:31
+ ->getName()
+ .find("enable_if") != StringRef::npos;
+ };
leanil wrote:
> aaron.ballman wrote:
> > This should be using
leanil marked 3 inline comments as done.
leanil added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:31
+ ->getName()
+ .find("enable_if") != StringRef::npos;
+ };
aaron.ballman wrote:
> This should
leanil updated this revision to Diff 91144.
leanil added a comment.
Fix enable_if detection in pointer and reference types.
Improve test coverage for these cases.
Repository:
rL LLVM
https://reviews.llvm.org/D30547
Files:
clang-tidy/misc/CMakeLists.txt
aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:31
+ ->getName()
+ .find("enable_if") !=
JonasToth added inline comments.
Comment at: docs/clang-tidy/checks/misc-forwarding-reference-overload.rst:38
+constructors. We suppress warnings if the copy and the move constructors are
both
+disabled (deleted or private), because there is nothing the prefect forwarding
leanil marked 6 inline comments as done.
leanil added inline comments.
Comment at: test/clang-tidy/misc-forwarding-reference-overload.cpp:21
+ Person(T &);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'Person' can hide copy
and move constructors
leanil updated this revision to Diff 90569.
leanil added a comment.
Added changes according to the comments.
Repository:
rL LLVM
https://reviews.llvm.org/D30547
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:23
+AST_MATCHER(QualType, isEnableIf) {
+ auto checkTemplate = [](const TemplateSpecializationType *Spec) {
+if (!Spec || !Spec->getTemplateName().getAsTemplateDecl()) {
JonasToth added a comment.
please add the check in the release notes as well.
https://reviews.llvm.org/D30547
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
JonasToth added a comment.
added my thoughts. i like the check, seems really thought out :)
Comment at: docs/clang-tidy/checks/misc-forwarding-reference-overload.rst:36
+
+The check warns for constructors C1 and C2, because those can act like
+described above. We suppress
leanil added inline comments.
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:22
+// Check if the given type is related to std::enable_if.
+AST_MATCHER(QualType, isEnableIf) {
+ auto checkTemplate = [](const TemplateSpecializationType *Spec) {
leanil created this revision.
leanil added a project: clang-tools-extra.
Herald added subscribers: JDevlieghere, mgorny.
Perfect forwarding constructors are called instead of copy constructors if the
forwarding reference provides a closer match (e.g. with non-const parameter).
This can be
33 matches
Mail list logo
