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:
> > leanil wrote:
> > > aaron.ballman wrote:
> > > > I think a better diagnostic might be: "constructor accepting a
> > > > universal reference hides the %select{copy|move|both the copy and
> > > > move}0 %select{constructor{|s}1"
> > > >
> > > > And then provide a note ("%select{copy|move}0 constructor declared
> > > > here") that points to the offending copy and/or move constructor.
> > > Without checking actual constructor calls, I would have to make notes on
> > > every (non disabled) copy / move constructor, any time I produce a
> > > warning. And as the warning already states where the problem lies, the
> > > notes would only help people find the copy and move constructors. Do you
> > > think that's necessary?
> > The warning states where the forwarding reference constructor is, but it
> > doesn't state where the conflicting constructors are. When we issue
> > diagnostics like that, we generally use notes so that the user can see all
> > of the locations involved -- the user may want to get rid of the other
> > constructors, or they may want to get rid of the forwarding reference
> > constructor. Also, saying "can hide" implies that it isn't hiding anything
> > at all, just that it's possible to do so. Tightening up the wording and
> > showing all of the locations involved solves both problems.
> This isn't quite complete. It's still an ambiguous statement to say "it can
> hide"; it does hide these constructors, and we even know which ones. Emit the
> notes before you emit the main diagnostic and you can use the `%select`
> suggested originally to be specific in the diagnostic.
We can not say for sure without looking at a concrete call whether a
constructor is "hidden" or not. It is always determined during the overload
resolution.
This check does not consider the calls, because that way it would always miss
the possible misuses if libraries.
Repository:
rL LLVM
https://reviews.llvm.org/D30547
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
