llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.75k stars 11.89k forks source link

Bogus reference bindings incorrectly accepted #44033

Open CaseyCarter opened 4 years ago

CaseyCarter commented 4 years ago
Bugzilla Link 44688
Version trunk
OS Windows NT
CC @DougGregor,@zygoloid

Extended Description

Today's trunk and release/10.x accept the TU:

  struct ExplicitToInt {
    explicit operator int() const;
  };

  const int& x(ExplicitToInt{}); // #1
  int&& y(ExplicitToInt{});      // #2

despite that GCC, MSVC, and I agree that both reference bindings are ill-formed. Running quickly through the bullets in [dcl.init.ref]/5:

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 4 years ago

Turns out that DR1758 is not actually broken, but fixing DR2267 exposes a longstanding infelicity in how we implement the rule that explicit conversion functions can sometimes be used when initializing a temporary bound to a copy/move constructor's reference parameter. Some yak shaving is required here...

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 4 years ago

Oh, this is unfortunate... implementing the DR2267 rule change breaks the example from DR1758:

struct X { X(); };
struct Y { explicit operator X(); } y;
X x{y};

(in pre-C++17 language modes) because we can no longer convert y when calling the move constructor of X.

Also of note: it turns out that MSVC and ICC don't reject the example in comment#0 if the target type of the conversion is a class type. (Clang's current behavior is consistent between classes and non-classes, but being consistently wrong isn't worth much.)

We should find a way to reject the comment#1 example (for both class and non-class destination types) without undoing DR1758 for C++11 and C++14. GCC manages it somehow.

CaseyCarter commented 4 years ago

Ah, thank you for the clarification about following the cross-reference to [over.match.ref]. I knew there had to be a reason that 5.1.2 and 5.3.2 simply say "can be converted" vs. 5.4.2's "is implicitly converted".

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 4 years ago
  • 5.3.2: doesn't apply, assuming that "can be converted" means "can be implicitly converted"

It doesn't exactly mean that. It has a cross-reference to 12.4.1.6 [over.match.ref], which contains the rules in scope here; those rules do permit using an explicit conversion function in the context of direct-initialization. However, those rules only consider conversion functions that convert to a reference type. So:

  struct ExplicitToInt {
    explicit operator int&() const;
    explicit operator int&&() const;
  };

  const int& x(ExplicitToInt{});
  int&& y(ExplicitToInt{});

... is valid, per /5.3.2. But the original testcase is not. I would imagine we're not restricting to conversion functions that convert to reference types in this case.

This is only invalid due to a recent rule change (DR2267) which Clang doesn't yet implement.