rsmith added a comment.

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. 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.

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).

Can you provide a list of the things this found in Qt and rethinkdb? Is there 
some pattern in them? Are they bugs?


================
Comment at: lib/Sema/SemaDeclCXX.cpp:4924-4929
@@ +4923,8 @@
+                                           bool Move) {
+  // Check that there are no ref-qualifiers.
+  if (Assign->getRefQualifier() != RQ_None) {
+    S.Diag(Assign->getLocation(), diag::warn_non_idiomatic_copy_move_assign)
+        << Move << /*ref-qualifier*/ 2;
+    return;
+  }
+
----------------
I don't think it makes sense to warn on this when the qualifier is `&`. In that 
case, the user's code is *better* than the normal idiom. And if they used the 
ref-qualifier `&&`, perhaps we should just assume they know what they're doing?


http://reviews.llvm.org/D15456



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to