Q on patch for CWG 2352

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Q on patch for CWG 2352

suyash singh via cfe-dev
Richard Smith made a patch in f041e9ad for CWG2352.
As a consequence of this patch, we had an obscure test failure,
and it's not clear to me that it's an intentional consequence.
So I figured I'd ask here.

Reduced test case:

bool foo(void * const * const *       &&) { return false; }
bool foo(void *       * const * const &)  { return true; }
bool bar() {
  return foo(reinterpret_cast<void***>(2));
}

Prior to the patch, the compiler selected the second overload;
after the patch, it selects the first overload.  Apparently there's
some subtle difference in the preferred-ness of one over the other,
and AFAICT the const-nesses aren't supposed to factor in any more?
so it's about the &-ref versus the &&-ref?

As I said, mainly I want to make sure this was intentional; if it
is, we can fiddle our test and that's the end of it.  But if it's
not intentional, this change is in the almost-final Clang 10.0
release, and might want to be fixed before it goes out.

Thanks,
--paulr

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Q on patch for CWG 2352

suyash singh via cfe-dev
[hidden email] for visibility.



On Wed, Mar 4, 2020 at 12:56 PM Robinson, Paul via cfe-dev <[hidden email]> wrote:
Richard Smith made a patch in f041e9ad for CWG2352.
As a consequence of this patch, we had an obscure test failure,
and it's not clear to me that it's an intentional consequence.
So I figured I'd ask here.

Reduced test case:

bool foo(void * const * const *       &&) { return false; }
bool foo(void *       * const * const &)  { return true; }
bool bar() {
  return foo(reinterpret_cast<void***>(2));
}

Prior to the patch, the compiler selected the second overload;
after the patch, it selects the first overload.  Apparently there's
some subtle difference in the preferred-ness of one over the other,
and AFAICT the const-nesses aren't supposed to factor in any more?
so it's about the &-ref versus the &&-ref?

As I said, mainly I want to make sure this was intentional; if it
is, we can fiddle our test and that's the end of it.  But if it's
not intentional, this change is in the almost-final Clang 10.0
release, and might want to be fixed before it goes out.

Thanks,
--paulr

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Q on patch for CWG 2352

suyash singh via cfe-dev

Also +Hans for the question about the release.

 

From: cfe-dev <[hidden email]> on behalf of cfe-dev <[hidden email]>
Reply-To: David Blaikie <[hidden email]>
Date: Wednesday, March 4, 2020 at 1:33 PM
To: "Robinson, Paul" <[hidden email]>, Richard Smith <[hidden email]>
Cc: cfe-dev <[hidden email]>
Subject: Re: [cfe-dev] Q on patch for CWG 2352

 

[hidden email] for visibility.

 

On Wed, Mar 4, 2020 at 12:56 PM Robinson, Paul via cfe-dev <[hidden email]> wrote:

Richard Smith made a patch in f041e9ad for CWG2352.
As a consequence of this patch, we had an obscure test failure,
and it's not clear to me that it's an intentional consequence.
So I figured I'd ask here.

Reduced test case:

bool foo(void * const * const *       &&) { return false; }
bool foo(void *       * const * const &)  { return true; }
bool bar() {
  return foo(reinterpret_cast<void***>(2));
}

Prior to the patch, the compiler selected the second overload;
after the patch, it selects the first overload.  Apparently there's
some subtle difference in the preferred-ness of one over the other,
and AFAICT the const-nesses aren't supposed to factor in any more?
so it's about the &-ref versus the &&-ref?

As I said, mainly I want to make sure this was intentional; if it
is, we can fiddle our test and that's the end of it.  But if it's
not intentional, this change is in the almost-final Clang 10.0
release, and might want to be fixed before it goes out.

Thanks,
--paulr

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Q on patch for CWG 2352

suyash singh via cfe-dev
> Richard Smith made a patch in f041e9ad for CWG2352.
> As a consequence of this patch, we had an obscure test failure,
> and it's not clear to me that it's an intentional consequence.
> So I figured I'd ask here.
>
> Reduced test case:

Slightly edited and expanded test case:

bool foo(void * const * const *       &&) { return false; } // new choice
bool foo(void *       * const * const &)  { return true; }  // old choice
bool bar() {
 return foo(reinterpret_cast<void***>(2));
}
// A couple of additional cases:
void*** p;
void*** q() { return p; }
bool func1() { return foo(p); } // old and new both call '&' overload
bool func2() { return foo(q()); } // old calls '&', new calls '&&'


So it looks like it matters whether the argument is an expression?
--paulr


> Prior to the patch, the compiler selected the second overload;
> after the patch, it selects the first overload.  Apparently there's
> some subtle difference in the preferred-ness of one over the other,
> and AFAICT the const-nesses aren't supposed to factor in any more?
> so it's about the &-ref versus the &&-ref?
>
> As I said, mainly I want to make sure this was intentional; if it
> is, we can fiddle our test and that's the end of it.  But if it's
> not intentional, this change is in the almost-final Clang 10.0
> release, and might want to be fixed before it goes out.
>
> Thanks,
> --paulr
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Q on patch for CWG 2352

suyash singh via cfe-dev
On Thu, 5 Mar 2020 at 11:41, Robinson, Paul via cfe-dev <[hidden email]> wrote:
> Richard Smith made a patch in f041e9ad for CWG2352.
> As a consequence of this patch, we had an obscure test failure,
> and it's not clear to me that it's an intentional consequence.
> So I figured I'd ask here.
>
> Reduced test case:

Slightly edited and expanded test case:

bool foo(void * const * const *       &&) { return false; } // new choice
bool foo(void *       * const * const &)  { return true; }  // old choice
bool bar() {
 return foo(reinterpret_cast<void***>(2));
}

I'm not entirely sure how we were coming up with the old answer, but the new answer is correct: binding an rvalue reference to an rvalue is preferred over binding an lvalue reference.
 
// A couple of additional cases:
void*** p;
void*** q() { return p; }
bool func1() { return foo(p); } // old and new both call '&' overload
bool func2() { return foo(q()); } // old calls '&', new calls '&&'

Here, q() is an rvalue, so we prefer binding an rvalue reference. p is an lvalue, so that rule doesn't apply and we prefer binding the lvalue reference because it binds to a less-qualified type.

I believe the new Clang behavior is following the rules described in C++ [over.ics.rank] paragraph 3.
 
So it looks like it matters whether the argument is an expression?
--paulr


> Prior to the patch, the compiler selected the second overload;
> after the patch, it selects the first overload.  Apparently there's
> some subtle difference in the preferred-ness of one over the other,
> and AFAICT the const-nesses aren't supposed to factor in any more?
> so it's about the &-ref versus the &&-ref?
>
> As I said, mainly I want to make sure this was intentional; if it
> is, we can fiddle our test and that's the end of it.  But if it's
> not intentional, this change is in the almost-final Clang 10.0
> release, and might want to be fixed before it goes out.
>
> Thanks,
> --paulr
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Q on patch for CWG 2352

suyash singh via cfe-dev

+Hans for 10.0 visibility.

 

From: Richard Smith <[hidden email]>
Reply-To: "[hidden email]" <[hidden email]>
Date: Friday, March 6, 2020 at 4:28 PM
To: "Robinson, Paul" <[hidden email]>
Cc: Shoaib Meenai <[hidden email]>, David Blaikie <[hidden email]>, cfe-dev <[hidden email]>
Subject: Re: [cfe-dev] Q on patch for CWG 2352

 

On Thu, 5 Mar 2020 at 11:41, Robinson, Paul via cfe-dev <[hidden email]> wrote:

> Richard Smith made a patch in f041e9ad for CWG2352.
> As a consequence of this patch, we had an obscure test failure,
> and it's not clear to me that it's an intentional consequence.
> So I figured I'd ask here.
>
> Reduced test case:

Slightly edited and expanded test case:

bool foo(void * const * const *       &&) { return false; } // new choice
bool foo(void *       * const * const &)  { return true; }  // old choice
bool bar() {
 return foo(reinterpret_cast<void***>(2));
}

 

I'm not entirely sure how we were coming up with the old answer, but the new answer is correct: binding an rvalue reference to an rvalue is preferred over binding an lvalue reference.

 

// A couple of additional cases:
void*** p;
void*** q() { return p; }
bool func1() { return foo(p); } // old and new both call '&' overload
bool func2() { return foo(q()); } // old calls '&', new calls '&&'

 

Here, q() is an rvalue, so we prefer binding an rvalue reference. p is an lvalue, so that rule doesn't apply and we prefer binding the lvalue reference because it binds to a less-qualified type.

 

I believe the new Clang behavior is following the rules described in C++ [over.ics.rank] paragraph 3.

 

So it looks like it matters whether the argument is an expression?
--paulr


> Prior to the patch, the compiler selected the second overload;
> after the patch, it selects the first overload.  Apparently there's
> some subtle difference in the preferred-ness of one over the other,
> and AFAICT the const-nesses aren't supposed to factor in any more?
> so it's about the &-ref versus the &&-ref?
>
> As I said, mainly I want to make sure this was intentional; if it
> is, we can fiddle our test and that's the end of it.  But if it's
> not intentional, this change is in the almost-final Clang 10.0
> release, and might want to be fixed before it goes out.
>
> Thanks,
> --paulr
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Q on patch for CWG 2352

suyash singh via cfe-dev

> +Hans for 10.0 visibility.

 

The new (corrected) behavior is in 10.0, there is nothing to fix here.  Arguably there could be a release note.

--paulr

 

From: Shoaib Meenai <[hidden email]>
Sent: Friday, March 6, 2020 7:35 PM
To: [hidden email]; Robinson, Paul <[hidden email]>
Cc: David Blaikie <[hidden email]>; [hidden email]; Hans Wennborg <[hidden email]>
Subject: Re: [cfe-dev] Q on patch for CWG 2352

 

+Hans for 10.0 visibility.

 

From: Richard Smith <[hidden email]>
Reply-To: "[hidden email]" <[hidden email]>
Date: Friday, March 6, 2020 at 4:28 PM
To: "Robinson, Paul" <[hidden email]>
Cc: Shoaib Meenai <[hidden email]>, David Blaikie <[hidden email]>, cfe-dev <[hidden email]>
Subject: Re: [cfe-dev] Q on patch for CWG 2352

 

On Thu, 5 Mar 2020 at 11:41, Robinson, Paul via cfe-dev <[hidden email]> wrote:

> Richard Smith made a patch in f041e9ad for CWG2352.
> As a consequence of this patch, we had an obscure test failure,
> and it's not clear to me that it's an intentional consequence.
> So I figured I'd ask here.
>
> Reduced test case:

Slightly edited and expanded test case:

bool foo(void * const * const *       &&) { return false; } // new choice
bool foo(void *       * const * const &)  { return true; }  // old choice
bool bar() {
 return foo(reinterpret_cast<void***>(2));
}

 

I'm not entirely sure how we were coming up with the old answer, but the new answer is correct: binding an rvalue reference to an rvalue is preferred over binding an lvalue reference.

 

// A couple of additional cases:
void*** p;
void*** q() { return p; }
bool func1() { return foo(p); } // old and new both call '&' overload
bool func2() { return foo(q()); } // old calls '&', new calls '&&'

 

Here, q() is an rvalue, so we prefer binding an rvalue reference. p is an lvalue, so that rule doesn't apply and we prefer binding the lvalue reference because it binds to a less-qualified type.

 

I believe the new Clang behavior is following the rules described in C++ [over.ics.rank] paragraph 3.

 

So it looks like it matters whether the argument is an expression?
--paulr


> Prior to the patch, the compiler selected the second overload;
> after the patch, it selects the first overload.  Apparently there's
> some subtle difference in the preferred-ness of one over the other,
> and AFAICT the const-nesses aren't supposed to factor in any more?
> so it's about the &-ref versus the &&-ref?
>
> As I said, mainly I want to make sure this was intentional; if it
> is, we can fiddle our test and that's the end of it.  But if it's
> not intentional, this change is in the almost-final Clang 10.0
> release, and might want to be fixed before it goes out.
>
> Thanks,
> --paulr
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Q on patch for CWG 2352

suyash singh via cfe-dev
Right. I was just bringing Richard's reply to Hans' attention, to make sure he saw that the new behavior was correct and there were no concerns here for the 10.0 release. 


From: Robinson, Paul <[hidden email]>
Sent: Monday, March 9, 2020 5:38:45 AM
To: Shoaib Meenai <[hidden email]>; [hidden email] <[hidden email]>
Cc: David Blaikie <[hidden email]>; [hidden email] <[hidden email]>; Hans Wennborg <[hidden email]>
Subject: RE: [cfe-dev] Q on patch for CWG 2352
 

> +Hans for 10.0 visibility.

 

The new (corrected) behavior is in 10.0, there is nothing to fix here.  Arguably there could be a release note.

--paulr

 

From: Shoaib Meenai <[hidden email]>
Sent: Friday, March 6, 2020 7:35 PM
To: [hidden email]; Robinson, Paul <[hidden email]>
Cc: David Blaikie <[hidden email]>; [hidden email]; Hans Wennborg <[hidden email]>
Subject: Re: [cfe-dev] Q on patch for CWG 2352

 

+Hans for 10.0 visibility.

 

From: Richard Smith <[hidden email]>
Reply-To: "[hidden email]" <[hidden email]>
Date: Friday, March 6, 2020 at 4:28 PM
To: "Robinson, Paul" <[hidden email]>
Cc: Shoaib Meenai <[hidden email]>, David Blaikie <[hidden email]>, cfe-dev <[hidden email]>
Subject: Re: [cfe-dev] Q on patch for CWG 2352

 

On Thu, 5 Mar 2020 at 11:41, Robinson, Paul via cfe-dev <[hidden email]> wrote:

> Richard Smith made a patch in f041e9ad for CWG2352.
> As a consequence of this patch, we had an obscure test failure,
> and it's not clear to me that it's an intentional consequence.
> So I figured I'd ask here.
>
> Reduced test case:

Slightly edited and expanded test case:

bool foo(void * const * const *       &&) { return false; } // new choice
bool foo(void *       * const * const &)  { return true; }  // old choice
bool bar() {
 return foo(reinterpret_cast<void***>(2));
}

 

I'm not entirely sure how we were coming up with the old answer, but the new answer is correct: binding an rvalue reference to an rvalue is preferred over binding an lvalue reference.

 

// A couple of additional cases:
void*** p;
void*** q() { return p; }
bool func1() { return foo(p); } // old and new both call '&' overload
bool func2() { return foo(q()); } // old calls '&', new calls '&&'

 

Here, q() is an rvalue, so we prefer binding an rvalue reference. p is an lvalue, so that rule doesn't apply and we prefer binding the lvalue reference because it binds to a less-qualified type.

 

I believe the new Clang behavior is following the rules described in C++ [over.ics.rank] paragraph 3.

 

So it looks like it matters whether the argument is an expression?
--paulr


> Prior to the patch, the compiler selected the second overload;
> after the patch, it selects the first overload.  Apparently there's
> some subtle difference in the preferred-ness of one over the other,
> and AFAICT the const-nesses aren't supposed to factor in any more?
> so it's about the &-ref versus the &&-ref?
>
> As I said, mainly I want to make sure this was intentional; if it
> is, we can fiddle our test and that's the end of it.  But if it's
> not intentional, this change is in the almost-final Clang 10.0
> release, and might want to be fixed before it goes out.
>
> Thanks,
> --paulr
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


From: Robinson, Paul <[hidden email]>
Sent: Monday, March 9, 2020 5:38:45 AM
To: Shoaib Meenai <[hidden email]>; [hidden email] <[hidden email]>
Cc: David Blaikie <[hidden email]>; [hidden email] <[hidden email]>; Hans Wennborg <[hidden email]>
Subject: RE: [cfe-dev] Q on patch for CWG 2352
 

> +Hans for 10.0 visibility.

 

The new (corrected) behavior is in 10.0, there is nothing to fix here.  Arguably there could be a release note.

--paulr

 

From: Shoaib Meenai <[hidden email]>
Sent: Friday, March 6, 2020 7:35 PM
To: [hidden email]; Robinson, Paul <[hidden email]>
Cc: David Blaikie <[hidden email]>; [hidden email]; Hans Wennborg <[hidden email]>
Subject: Re: [cfe-dev] Q on patch for CWG 2352

 

+Hans for 10.0 visibility.

 

From: Richard Smith <[hidden email]>
Reply-To: "[hidden email]" <[hidden email]>
Date: Friday, March 6, 2020 at 4:28 PM
To: "Robinson, Paul" <[hidden email]>
Cc: Shoaib Meenai <[hidden email]>, David Blaikie <[hidden email]>, cfe-dev <[hidden email]>
Subject: Re: [cfe-dev] Q on patch for CWG 2352

 

On Thu, 5 Mar 2020 at 11:41, Robinson, Paul via cfe-dev <[hidden email]> wrote:

> Richard Smith made a patch in f041e9ad for CWG2352.
> As a consequence of this patch, we had an obscure test failure,
> and it's not clear to me that it's an intentional consequence.
> So I figured I'd ask here.
>
> Reduced test case:

Slightly edited and expanded test case:

bool foo(void * const * const *       &&) { return false; } // new choice
bool foo(void *       * const * const &)  { return true; }  // old choice
bool bar() {
 return foo(reinterpret_cast<void***>(2));
}

 

I'm not entirely sure how we were coming up with the old answer, but the new answer is correct: binding an rvalue reference to an rvalue is preferred over binding an lvalue reference.

 

// A couple of additional cases:
void*** p;
void*** q() { return p; }
bool func1() { return foo(p); } // old and new both call '&' overload
bool func2() { return foo(q()); } // old calls '&', new calls '&&'

 

Here, q() is an rvalue, so we prefer binding an rvalue reference. p is an lvalue, so that rule doesn't apply and we prefer binding the lvalue reference because it binds to a less-qualified type.

 

I believe the new Clang behavior is following the rules described in C++ [over.ics.rank] paragraph 3.

 

So it looks like it matters whether the argument is an expression?
--paulr


> Prior to the patch, the compiler selected the second overload;
> after the patch, it selects the first overload.  Apparently there's
> some subtle difference in the preferred-ness of one over the other,
> and AFAICT the const-nesses aren't supposed to factor in any more?
> so it's about the &-ref versus the &&-ref?
>
> As I said, mainly I want to make sure this was intentional; if it
> is, we can fiddle our test and that's the end of it.  But if it's
> not intentional, this change is in the almost-final Clang 10.0
> release, and might want to be fixed before it goes out.
>
> Thanks,
> --paulr
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev