[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive

[Nicolas Lesser via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337017: Fix PR34668 - P0704R1 implementation is too 
permissive (authored by Rakete, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D38075?vs=155410=155412#toc

Repository:
  rC Clang

https://reviews.llvm.org/D38075

Files:
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp


Index: test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
===
--- test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
+++ test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
@@ -3,12 +3,15 @@
 struct X {
   void ref() & {} // expected-note{{'ref' declared here}}
   void cref() const& {}
+  void cvref() const volatile & {} // expected-note{{'cvref' declared here}}
 };
 
 void test() {
   X{}.ref(); // expected-error{{'this' argument to member function 'ref' is an 
rvalue, but function has non-const lvalue ref-qualifier}}
   X{}.cref(); // expected-no-error
+  X{}.cvref(); // expected-error{{'this' argument to member function 'cvref' 
is an rvalue, but function has non-const lvalue ref-qualifier}}
 
   (X{}.*::ref)(); // expected-error-re{{pointer-to-member function type 
'void (X::*)() {{.*}}&' can only be called on an lvalue}}
   (X{}.*::cref)(); // expected-no-error
+  (X{}.*::cvref)(); // expected-error-re{{pointer-to-member function type 
'void (X::*)() {{.*}}&' can only be called on an lvalue}}
 }
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5472,8 +5472,9 @@
 
 case RQ_LValue:
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
-// C++2a allows functions with ref-qualifier & if they are also 
'const'.
-if (Proto->isConst())
+// C++2a allows functions with ref-qualifier & if their 
cv-qualifier-seq
+// is (exactly) 'const'.
+if (Proto->isConst() && !Proto->isVolatile())
   Diag(Loc, getLangOpts().CPlusPlus2a
 ? 
diag::warn_cxx17_compat_pointer_to_const_ref_member_on_rvalue
 : diag::ext_pointer_to_const_ref_member_on_rvalue);


Index: test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
===
--- test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
+++ test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
@@ -3,12 +3,15 @@
 struct X {
   void ref() & {} // expected-note{{'ref' declared here}}
   void cref() const& {}
+  void cvref() const volatile & {} // expected-note{{'cvref' declared here}}
 };
 
 void test() {
   X{}.ref(); // expected-error{{'this' argument to member function 'ref' is an rvalue, but function has non-const lvalue ref-qualifier}}
   X{}.cref(); // expected-no-error
+  X{}.cvref(); // expected-error{{'this' argument to member function 'cvref' is an rvalue, but function has non-const lvalue ref-qualifier}}
 
   (X{}.*::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}}
   (X{}.*::cref)(); // expected-no-error
+  (X{}.*::cvref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}}
 }
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5472,8 +5472,9 @@
 
 case RQ_LValue:
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
-// C++2a allows functions with ref-qualifier & if they are also 'const'.
-if (Proto->isConst())
+// C++2a allows functions with ref-qualifier & if their cv-qualifier-seq
+// is (exactly) 'const'.
+if (Proto->isConst() && !Proto->isVolatile())
   Diag(Loc, getLangOpts().CPlusPlus2a
 ? diag::warn_cxx17_compat_pointer_to_const_ref_member_on_rvalue
 : diag::ext_pointer_to_const_ref_member_on_rvalue);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 155410.
Rakete added a comment.

Change error message to reflect the new update error message since the last 
revision.


Repository:
  rC Clang

https://reviews.llvm.org/D38075

Files:
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp


Index: test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
===
--- test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
+++ test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
@@ -3,12 +3,15 @@
 struct X {
   void ref() & {} // expected-note{{'ref' declared here}}
   void cref() const& {}
+  void cvref() const volatile & {} // expected-note{{'cvref' declared here}}
 };
 
 void test() {
   X{}.ref(); // expected-error{{'this' argument to member function 'ref' is an 
rvalue, but function has non-const lvalue ref-qualifier}}
   X{}.cref(); // expected-no-error
+  X{}.cvref(); // expected-error{{'this' argument to member function 'cvref' 
is an rvalue, but function has non-const lvalue ref-qualifier}}
 
   (X{}.*::ref)(); // expected-error-re{{pointer-to-member function type 
'void (X::*)() {{.*}}&' can only be called on an lvalue}}
   (X{}.*::cref)(); // expected-no-error
+  (X{}.*::cvref)(); // expected-error-re{{pointer-to-member function type 
'void (X::*)() {{.*}}&' can only be called on an lvalue}}
 }
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5472,8 +5472,9 @@
 
 case RQ_LValue:
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
-// C++2a allows functions with ref-qualifier & if they are also 
'const'.
-if (Proto->isConst())
+// C++2a allows functions with ref-qualifier & if their 
cv-qualifier-seq
+// is (exactly) 'const'.
+if (Proto->isConst() && !Proto->isVolatile())
   Diag(Loc, getLangOpts().CPlusPlus2a
 ? 
diag::warn_cxx17_compat_pointer_to_const_ref_member_on_rvalue
 : diag::ext_pointer_to_const_ref_member_on_rvalue);


Index: test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
===
--- test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
+++ test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
@@ -3,12 +3,15 @@
 struct X {
   void ref() & {} // expected-note{{'ref' declared here}}
   void cref() const& {}
+  void cvref() const volatile & {} // expected-note{{'cvref' declared here}}
 };
 
 void test() {
   X{}.ref(); // expected-error{{'this' argument to member function 'ref' is an rvalue, but function has non-const lvalue ref-qualifier}}
   X{}.cref(); // expected-no-error
+  X{}.cvref(); // expected-error{{'this' argument to member function 'cvref' is an rvalue, but function has non-const lvalue ref-qualifier}}
 
   (X{}.*::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}}
   (X{}.*::cref)(); // expected-no-error
+  (X{}.*::cvref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}}
 }
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5472,8 +5472,9 @@
 
 case RQ_LValue:
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
-// C++2a allows functions with ref-qualifier & if they are also 'const'.
-if (Proto->isConst())
+// C++2a allows functions with ref-qualifier & if their cv-qualifier-seq
+// is (exactly) 'const'.
+if (Proto->isConst() && !Proto->isVolatile())
   Diag(Loc, getLangOpts().CPlusPlus2a
 ? diag::warn_cxx17_compat_pointer_to_const_ref_member_on_rvalue
 : diag::ext_pointer_to_const_ref_member_on_rvalue);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive

[Tim Song via Phabricator via cfe-commits]

tcanens added a comment.

In https://reviews.llvm.org/D38075#1161325, @Rakete wrote:

> Do you need someone to commit it?


Yes, please.


https://reviews.llvm.org/D38075



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

[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete accepted this revision.
Rakete added a comment.
This revision is now accepted and ready to land.

LGTM with a small change in the error message that needs fixing.

Do you need someone to commit it?


https://reviews.llvm.org/D38075



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

[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive

[Tim Song via Phabricator via cfe-commits]

tcanens updated this revision to Diff 116490.
tcanens added a comment.

Also tweaked preceding comment.


https://reviews.llvm.org/D38075

Files:
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp


Index: test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
===
--- test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
+++ test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
@@ -3,12 +3,15 @@
 struct X {
   void ref() & {}
   void cref() const& {}
+  void cvref() const volatile& {}
 };
 
 void test() {
   X{}.ref(); // expected-error{{cannot initialize object parameter of type 'X' 
with an expression of type 'X'}}
   X{}.cref(); // expected-no-error
+  X{}.cvref(); // expected-error{{cannot initialize object parameter of type 
'const volatile X' with an expression of type 'X'}}
 
   (X{}.*::ref)(); // expected-error-re{{pointer-to-member function type 
'void (X::*)() {{.*}}&' can only be called on an lvalue}}
   (X{}.*::cref)(); // expected-no-error
+  (X{}.*::cvref)(); // expected-error-re{{pointer-to-member function type 
'void (X::*)() {{.*}}&' can only be called on an lvalue}}
 }
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5182,8 +5182,9 @@
 
 case RQ_LValue:
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
-// C++2a allows functions with ref-qualifier & if they are also 
'const'.
-if (Proto->isConst())
+// C++2a allows functions with ref-qualifier & if their 
cv-qualifier-seq
+// is (exactly) 'const'.
+if (Proto->isConst() && !Proto->isVolatile())
   Diag(Loc, getLangOpts().CPlusPlus2a
 ? 
diag::warn_cxx17_compat_pointer_to_const_ref_member_on_rvalue
 : diag::ext_pointer_to_const_ref_member_on_rvalue);


Index: test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
===
--- test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
+++ test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
@@ -3,12 +3,15 @@
 struct X {
   void ref() & {}
   void cref() const& {}
+  void cvref() const volatile& {}
 };
 
 void test() {
   X{}.ref(); // expected-error{{cannot initialize object parameter of type 'X' with an expression of type 'X'}}
   X{}.cref(); // expected-no-error
+  X{}.cvref(); // expected-error{{cannot initialize object parameter of type 'const volatile X' with an expression of type 'X'}}
 
   (X{}.*::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}}
   (X{}.*::cref)(); // expected-no-error
+  (X{}.*::cvref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}}
 }
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5182,8 +5182,9 @@
 
 case RQ_LValue:
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
-// C++2a allows functions with ref-qualifier & if they are also 'const'.
-if (Proto->isConst())
+// C++2a allows functions with ref-qualifier & if their cv-qualifier-seq
+// is (exactly) 'const'.
+if (Proto->isConst() && !Proto->isVolatile())
   Diag(Loc, getLangOpts().CPlusPlus2a
 ? diag::warn_cxx17_compat_pointer_to_const_ref_member_on_rvalue
 : diag::ext_pointer_to_const_ref_member_on_rvalue);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added inline comments.



Comment at: lib/Sema/SemaExprCXX.cpp:5185
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
 // C++2a allows functions with ref-qualifier & if they are also 
'const'.
+if (Proto->isConst() && !Proto->isVolatile())

tcanens wrote:
> Rakete wrote:
> > I think you should update the comment to something like "also 'const', but 
> > not if they're 'volatile'."
> "if their cv-qualifier-seq is (exactly) 'const'", maybe?
Sure that works too :)


https://reviews.llvm.org/D38075



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

[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive

[Tim Song via Phabricator via cfe-commits]

tcanens added inline comments.



Comment at: lib/Sema/SemaExprCXX.cpp:5185
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
 // C++2a allows functions with ref-qualifier & if they are also 
'const'.
+if (Proto->isConst() && !Proto->isVolatile())

Rakete wrote:
> I think you should update the comment to something like "also 'const', but 
> not if they're 'volatile'."
"if their cv-qualifier-seq is (exactly) 'const'", maybe?


https://reviews.llvm.org/D38075



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

[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added a comment.

Thanks! That was an oversight on my part, sorry.




Comment at: lib/Sema/SemaExprCXX.cpp:5185
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
 // C++2a allows functions with ref-qualifier & if they are also 
'const'.
+if (Proto->isConst() && !Proto->isVolatile())

I think you should update the comment to something like "also 'const', but not 
if they're 'volatile'."


https://reviews.llvm.org/D38075



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

[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive

[Tim Song via Phabricator via cfe-commits]

tcanens created this revision.
tcanens added a project: clang.

https://bugs.llvm.org/show_bug.cgi?id=34668

Pretty straightforward.


Repository:
  rL LLVM

https://reviews.llvm.org/D38075

Files:
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp


Index: test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
===
--- test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
+++ test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
@@ -3,12 +3,15 @@
 struct X {
   void ref() & {}
   void cref() const& {}
+  void cvref() const volatile& {}
 };
 
 void test() {
   X{}.ref(); // expected-error{{cannot initialize object parameter of type 'X' 
with an expression of type 'X'}}
   X{}.cref(); // expected-no-error
+  X{}.cvref(); // expected-error{{cannot initialize object parameter of type 
'const volatile X' with an expression of type 'X'}}
 
   (X{}.*::ref)(); // expected-error-re{{pointer-to-member function type 
'void (X::*)() {{.*}}&' can only be called on an lvalue}}
   (X{}.*::cref)(); // expected-no-error
+  (X{}.*::cvref)(); // expected-error-re{{pointer-to-member function type 
'void (X::*)() {{.*}}&' can only be called on an lvalue}}
 }
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5183,7 +5183,7 @@
 case RQ_LValue:
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
 // C++2a allows functions with ref-qualifier & if they are also 
'const'.
-if (Proto->isConst())
+if (Proto->isConst() && !Proto->isVolatile())
   Diag(Loc, getLangOpts().CPlusPlus2a
 ? 
diag::warn_cxx17_compat_pointer_to_const_ref_member_on_rvalue
 : diag::ext_pointer_to_const_ref_member_on_rvalue);


Index: test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
===
--- test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
+++ test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp
@@ -3,12 +3,15 @@
 struct X {
   void ref() & {}
   void cref() const& {}
+  void cvref() const volatile& {}
 };
 
 void test() {
   X{}.ref(); // expected-error{{cannot initialize object parameter of type 'X' with an expression of type 'X'}}
   X{}.cref(); // expected-no-error
+  X{}.cvref(); // expected-error{{cannot initialize object parameter of type 'const volatile X' with an expression of type 'X'}}
 
   (X{}.*::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}}
   (X{}.*::cref)(); // expected-no-error
+  (X{}.*::cvref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}}
 }
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5183,7 +5183,7 @@
 case RQ_LValue:
   if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
 // C++2a allows functions with ref-qualifier & if they are also 'const'.
-if (Proto->isConst())
+if (Proto->isConst() && !Proto->isVolatile())
   Diag(Loc, getLangOpts().CPlusPlus2a
 ? diag::warn_cxx17_compat_pointer_to_const_ref_member_on_rvalue
 : diag::ext_pointer_to_const_ref_member_on_rvalue);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits