Fix PR34668 - P0704R1 implementation is too permissive

[tcanens]

Summary

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

Pretty straightforward.

[Rakete1111]

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

lib/Sema/SemaExprCXX.cpp

5185

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

[tcanens]

lib/Sema/SemaExprCXX.cpp

5185

"if their cv-qualifier-seq is (exactly) 'const'", maybe?

[Rakete1111]

lib/Sema/SemaExprCXX.cpp

5185

Sure that works too :)

[tcanens]

Also tweaked preceding comment.

[Rakete1111]

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

Do you need someone to commit it?

[tcanens]

In D38075#1161325, @Rakete1111 wrote:

Do you need someone to commit it?

Yes, please.

[Rakete1111]

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

lib/Sema/SemaExprCXX.cpp

case BTT_IsTriviallyAssignable: {
     if (!RhsT->isVoidType() && !RhsT->isIncompleteArrayType() &&
         Self.RequireCompleteType(KeyLoc, RhsT,
           diag::err_incomplete_type_used_in_type_trait_expr))
       return false;
 
     // cv void is never assignable.
     if (LhsT->isVoidType() || RhsT->isVoidType())
       return false;
 
     // Build expressions that emulate the effect of declval<T>() and
     // declval<U>().
     if (LhsT->isObjectType() || LhsT->isFunctionType())
       LhsT = Self.Context.getRValueReferenceType(LhsT);
     if (RhsT->isObjectType() || RhsT->isFunctionType())
       RhsT = Self.Context.getRValueReferenceType(RhsT);
     OpaqueValueExpr Lhs(KeyLoc, LhsT.getNonLValueExprType(Self.Context),
                         Expr::getValueKindForType(LhsT));
QualType Sema::CheckPointerToMemberOperands(ExprResult &LHS, ExprResult &RHS,
   if (const FunctionProtoType *Proto = Result->getAs<FunctionProtoType>()) {
     switch (Proto->getRefQualifier()) {
     case RQ_None:
       // Do nothing
       break;
 
     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);
         else
           Diag(Loc, diag::err_pointer_to_member_oper_value_classify)
               << RHSType << 1 << LHS.get()->getSourceRange();
       }
       break;

test/SemaCXX/cxx2a-pointer-to-const-ref-member.cpp

 // RUN: %clang_cc1 -std=c++2a %s -verify
 
 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}}
 }