aaron.ballman added a comment.
In http://reviews.llvm.org/D15456#320946, @rsmith wrote:
> I'm unconvinced this meets the bar for an on-by-default warning. Your
> suggested workaround for correct-but-diagnosed code doesn't seem to work: if
> I have
>
> struct X {
> X(const volatile X&);
> };
>
>
> ... adding
>
> X(const X&) = delete;
>
>
> or a private "normal" copy constructor will break clients of my code that
> happen to have a non-volatile `X` object.
That's a good point. You could use delegating constructors to silence the
warning, but that is a bit intrusive (and non-functional for coding targeting
older versions of the standard).
> This will also diagnose the pre-C++11 idiom of:
>
> struct noncopyable {
> noncopyable(noncopyable&);
> void operator=(noncopyable&);
> };
>
>
> ... where the references are deliberately references to non-const in order to
> allow more bugs to be caught at compile time.
This was intentional, based on discussion. The thought was that (1) these are
still copy constructible/assignable (within the context of the class type
itself), and (2) they're still not idiomatic. However, I do think that's a bit
of a weak argument and would be fine allowing those cases.
> I'd also expect that if the user wrote more tokens than would be present in
> the idiomatic declaration, they probably know what they're doing (so don't
> reject extra cv-qualifiers and ref-qualifiers).
It would be oddly inconsistent to warn on some non-idiomatic operations, but
then fail to warn on other non-idiomatic operations though. However, it may
make sense
> Can you provide a list of the things this found in Qt and rethinkdb? Is there
> some pattern in them? Are they bugs?
The Qt issues are things like:
http://code.qt.io/cgit/qt/qtbase.git/tree/src/tools/uic/ui4.h?h=dev#n379
However, there are also plenty like:
dom/QualifiedName.h:70:11: warning: non-idiomatic copy assignment operator
declaration; consider returning 'T&' instead [-Wnon-idiomatic-copy-move]
const QualifiedName& operator=(const QualifiedName& other) { other.ref();
deref(); m_impl = other.m_impl; return *this; }
Rethink's look more like:
http://reviews.llvm.org/D15456
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
