[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive
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
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
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{}.*&X::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}} (X{}.*&X::cref)(); // expected-no-error + (X{}.*&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{}.*&X::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}} (X{}.*&X::cref)(); // expected-no-error + (X{}.*&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
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&id=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{}.*&X::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}} (X{}.*&X::cref)(); // expected-no-error + (X{}.*&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{}.*&X::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}} (X{}.*&X::cref)(); // expected-no-error + (X{}.*&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
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{}.*&X::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}} (X{}.*&X::cref)(); // expected-no-error + (X{}.*&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{}.*&X::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}} (X{}.*&X::cref)(); // expected-no-error + (X{}.*&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
[PATCH] D38075: Fix PR34668 - P0704R1 implementation is too permissive
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
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
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
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{}.*&X::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}} (X{}.*&X::cref)(); // expected-no-error + (X{}.*&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{}.*&X::ref)(); // expected-error-re{{pointer-to-member function type 'void (X::*)() {{.*}}&' can only be called on an lvalue}} (X{}.*&X::cref)(); // expected-no-error + (X{}.*&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