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 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.
I can see the logic in that. I guess I'm thinking of it the same way we use the
phrase "hidden" when describing code like:
```
struct Base {
virtual void f(int);
};
struct Derived : Base {
void f(double);
};
```
We claim Derived::f() hides Base::f() without considering the callers.
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
