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 is the most precise way to formulate the warning message I could come up
> with.
> The condition for excluding either "copy" or "move" from the warning is to
> find only disabled instances of the constructor type, and there must be at
> least one, otherwise the compiler generated constructor (which is not present
> in this enumeration) can be hidden.
How about:
```
if (OtherCtor->isDeleted() || OtherCtor->getAccess() == AS_private)
(OtherCtor->isCopyConstructor() ? DisabledCopy : DisabledMove) = true;
else
(OtherCtor->isCopyConstructor() ? EnabledCopy : EnabledMove) = true;
```
================
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:136
+ NoMove = DisabledMove && !EnabledMove;
+ if (NoCopy && NoMove) {
+ return;
----------------
Can elide the braces.
================
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:140
+ diag(Ctor->getLocation(), "constructor accepting a forwarding reference can "
+ "hide the %select{copy|move|copy and the "
+ "move}0 constructor%s1")
----------------
copy and the move -> copy and move
================
Comment at: clang-tidy/misc/ForwardingReferenceOverloadCheck.cpp:142
+ "move}0 constructor%s1")
+ << 2 - NoCopy - 2 * NoMove << NoCopy + NoMove;
+ for (const auto *OtherCtor : Ctor->getParent()->ctors()) {
----------------
The math is correct, but really makes me scratch my head to try to puzzle
through it. Why not something along the lines of `Copy && Move ? 2 : (Copy ? 0
: 1)`
Repository:
rL LLVM
https://reviews.llvm.org/D30547
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
