[Clang] Implement CWG2813: Class member access with prvalues

clang-tools-extra/clangd/unittests/DumpASTTests.cpp

@@ -49,7 +49,7 @@ declaration: Function - root
       )"},
       {R"cpp(
 namespace root {
-struct S { static const int x = 0; };
+struct S { static const int x = 0; ~S(); };
 int y = S::x + root::S().x;
 }
       )cpp",
@@ -60,10 +60,12 @@ declaration: Namespace - root
       type: Qualified - const
         type: Builtin - int
       expression: IntegerLiteral - 0
+    declaration: CXXDestructor
+      type: Record - S
+      type: FunctionProto
+        type: Builtin - void
     declaration: CXXConstructor
     declaration: CXXConstructor
-    declaration: CXXConstructor
-    declaration: CXXDestructor
   declaration: Var - y
     type: Builtin - int
     expression: ExprWithCleanups
@@ -74,14 +76,45 @@ declaration: Namespace - root
               type: Record - S
         expression: ImplicitCast - LValueToRValue
           expression: Member - x
-            expression: MaterializeTemporary - rvalue
+            expression: CXXBindTemporary
               expression: CXXTemporaryObject - S
                 type: Elaborated
                   specifier: Namespace - root::
                   type: Record - S
       )"},
       {R"cpp(
 namespace root {
+struct S { static const int x = 0; };
+int y = S::x + root::S().x;
+}
+      )cpp",
+       R"(
+declaration: Namespace - root
+  declaration: CXXRecord - S
+    declaration: Var - x
+      type: Qualified - const
+        type: Builtin - int
+      expression: IntegerLiteral - 0
+    declaration: CXXConstructor
+    declaration: CXXConstructor
+    declaration: CXXConstructor
+    declaration: CXXDestructor
+  declaration: Var - y
+    type: Builtin - int
+    expression: BinaryOperator - +
+      expression: ImplicitCast - LValueToRValue
+        expression: DeclRef - x
+          specifier: TypeSpec
+            type: Record - S
+      expression: ImplicitCast - LValueToRValue
+        expression: Member - x
+          expression: CXXTemporaryObject - S
+            type: Elaborated
+              specifier: Namespace - root::
+              type: Record - S
+      )"},
+      {R"cpp(
+namespace root {
 template <typename T> int tmpl() {
   (void)tmpl<unsigned>();
   return T::value;

clang/docs/ReleaseNotes.rst

@@ -260,6 +260,11 @@ Resolutions to C++ Defect Reports
 - Clang now requires a template argument list after a template keyword.
   (`CWG96: Syntactic disambiguation using the template keyword <https://cplusplus.github.io/CWG/issues/96.html>`_).
 
+- Clang now allows calling explicit object member functions directly with prvalues
+  instead of always materializing a temporary, meaning by-value explicit object parameters
+  do not need to move from a temporary.
+  (`CWG2813: Class member access with prvalues <https://cplusplus.github.io/CWG/issues/2813.html>`_).
+
 C Language Changes
 ------------------
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -9182,6 +9182,9 @@ def warn_unused_constructor : Warning<
 def warn_unused_constructor_msg : Warning<
   "ignoring temporary created by a constructor declared with %0 attribute: %1">,
   InGroup<UnusedValue>;
+def warn_discarded_class_member_access : Warning<
+  "left operand of dot in this class member access is discarded and has no effect">,
+  InGroup<UnusedValue>;
 def warn_side_effects_unevaluated_context : Warning<
   "expression with side effects has no effect in an unevaluated context">,
   InGroup<UnevaluatedExpression>;

clang/lib/Sema/SemaExprMember.cpp

@@ -1015,15 +1015,6 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
               : !isDependentScopeSpecifier(SS) || computeDeclContext(SS)) &&
          "dependent lookup context that isn't the current instantiation?");
 
-  // C++1z [expr.ref]p2:
-  //   For the first option (dot) the first expression shall be a glvalue [...]
-  if (!IsArrow && BaseExpr && BaseExpr->isPRValue()) {
-    ExprResult Converted = TemporaryMaterializationConversion(BaseExpr);
-    if (Converted.isInvalid())
-      return ExprError();
-    BaseExpr = Converted.get();
-  }
-
   const DeclarationNameInfo &MemberNameInfo = R.getLookupNameInfo();
   DeclarationName MemberName = MemberNameInfo.getName();
   SourceLocation MemberLoc = MemberNameInfo.getLoc();
@@ -1140,26 +1131,68 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
     BaseExpr = BuildCXXThisExpr(Loc, BaseExprType, /*IsImplicit=*/true);
   }
 
+  // C++17 [expr.ref]p2, per CWG2813:
+  //   For the first option (dot), if the id-expression names a static member or
+  //   an enumerator, the first expression is a discarded-value expression; if
+  //   the id-expression names a non-static data member, the first expression
+  //   shall be a glvalue.
+  auto MakeDiscardedValue = [&BaseExpr, IsArrow, this] {
+    assert(getLangOpts().CPlusPlus &&
+           "Static member / member enumerator outside of C++");
+    if (IsArrow)
+      return false;
+    ExprResult Converted = IgnoredValueConversions(BaseExpr);
+    if (Converted.isInvalid())
+      return true;
+    BaseExpr = Converted.get();
+    DiagnoseUnusedExprResult(BaseExpr,
+                             diag::warn_discarded_class_member_access);
+    return false;
+  };
+  auto MakeGLValue = [&BaseExpr, IsArrow, this] {
+    if (IsArrow || !BaseExpr->isPRValue())
+      return false;
+    ExprResult Converted = TemporaryMaterializationConversion(BaseExpr);
+    if (Converted.isInvalid())
+      return true;
+    BaseExpr = Converted.get();
+    return false;
+  };
+
   // Check the use of this member.
   if (DiagnoseUseOfDecl(MemberDecl, MemberLoc))
     return ExprError();
 
-  if (FieldDecl *FD = dyn_cast<FieldDecl>(MemberDecl))
+  if (FieldDecl *FD = dyn_cast<FieldDecl>(MemberDecl)) {
+    if (MakeGLValue())
+      return ExprError();
     return BuildFieldReferenceExpr(BaseExpr, IsArrow, OpLoc, SS, FD, FoundDecl,
                                    MemberNameInfo);
+  }
 
-  if (MSPropertyDecl *PD = dyn_cast<MSPropertyDecl>(MemberDecl))
+  if (MSPropertyDecl *PD = dyn_cast<MSPropertyDecl>(MemberDecl)) {
+    // Properties treated as non-static data members for the purpose of
+    // temporary materialization
+    if (MakeGLValue())
+      return ExprError();
     return BuildMSPropertyRefExpr(*this, BaseExpr, IsArrow, SS, PD,
                                   MemberNameInfo);
+  }
 
-  if (IndirectFieldDecl *FD = dyn_cast<IndirectFieldDecl>(MemberDecl))
+  if (IndirectFieldDecl *FD = dyn_cast<IndirectFieldDecl>(MemberDecl)) {
+    if (MakeGLValue())
+      return ExprError();
     // We may have found a field within an anonymous union or struct
     // (C++ [class.union]).
     return BuildAnonymousStructUnionMemberReference(SS, MemberLoc, FD,
                                                     FoundDecl, BaseExpr,
                                                     OpLoc);
+  }
 
+  // Static data member
   if (VarDecl *Var = dyn_cast<VarDecl>(MemberDecl)) {
+    if (MakeDiscardedValue())
+      return ExprError();
     return BuildMemberExpr(BaseExpr, IsArrow, OpLoc,
                            SS.getWithLocInContext(Context), TemplateKWLoc, Var,
                            FoundDecl, /*HadMultipleCandidates=*/false,
@@ -1174,6 +1207,9 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
       valueKind = VK_PRValue;
       type = Context.BoundMemberTy;
     } else {
+      // Static member function
+      if (MakeDiscardedValue())
+        return ExprError();
       valueKind = VK_LValue;
       type = MemberFn->getType();
     }
@@ -1186,13 +1222,17 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
   assert(!isa<FunctionDecl>(MemberDecl) && "member function not C++ method?");
 
   if (EnumConstantDecl *Enum = dyn_cast<EnumConstantDecl>(MemberDecl)) {
+    if (MakeDiscardedValue())
+      return ExprError();
     return BuildMemberExpr(
         BaseExpr, IsArrow, OpLoc, SS.getWithLocInContext(Context),
         TemplateKWLoc, Enum, FoundDecl, /*HadMultipleCandidates=*/false,
         MemberNameInfo, Enum->getType(), VK_PRValue, OK_Ordinary);
   }
 
   if (VarTemplateDecl *VarTempl = dyn_cast<VarTemplateDecl>(MemberDecl)) {
+    if (MakeDiscardedValue())
+      return ExprError();
     if (!TemplateArgs) {
       diagnoseMissingTemplateArguments(
           SS, /*TemplateKeyword=*/TemplateKWLoc.isValid(), VarTempl, MemberLoc);

clang/lib/Sema/SemaOverload.cpp

@@ -5922,7 +5922,9 @@ ExprResult Sema::PerformImplicitObjectArgumentInitialization(
     DestType = ImplicitParamRecordType;
     FromClassification = From->Classify(Context);
 
-    // When performing member access on a prvalue, materialize a temporary.
+    // CWG2813 [expr.call]p6:
+    //   If the function is an implicit object member function, the object
+    //   expression of the class member access shall be a glvalue [...]
     if (From->isPRValue()) {
       From = CreateMaterializeTemporaryExpr(FromRecordType, From,
                                             Method->getRefQualifier() !=
@@ -6457,11 +6459,6 @@ static Expr *GetExplicitObjectExpr(Sema &S, Expr *Obj,
                                 VK_LValue, OK_Ordinary, SourceLocation(),
                                 /*CanOverflow=*/false, FPOptionsOverride());
   }
-  if (Obj->Classify(S.getASTContext()).isPRValue()) {
-    Obj = S.CreateMaterializeTemporaryExpr(
-        ObjType, Obj,
-        !Fun->getParamDecl(0)->getType()->isRValueReferenceType());
-  }
   return Obj;
 }
 
@@ -15709,8 +15706,6 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, Expr *MemExprE,
                          CurFPFeatureOverrides(), Proto->getNumParams());
   } else {
     // Convert the object argument (for a non-static member function call).
-    // We only need to do this if there was actually an overload; otherwise
-    // it was done at lookup.
     ExprResult ObjectArg = PerformImplicitObjectArgumentInitialization(
         MemExpr->getBase(), Qualifier, FoundDecl, Method);
     if (ObjectArg.isInvalid())

clang/lib/Sema/SemaStmt.cpp

@@ -223,6 +223,7 @@ static bool DiagnoseNoDiscard(Sema &S, const WarnUnusedResultAttr *A,
 }
 
 void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
+  const unsigned OrigDiagID = DiagID;
   if (const LabelStmt *Label = dyn_cast_or_null<LabelStmt>(S))
     return DiagnoseUnusedExprResult(Label->getSubStmt(), DiagID);
 
@@ -387,9 +388,16 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
   // Do not diagnose use of a comma operator in a SFINAE context because the
   // type of the left operand could be used for SFINAE, so technically it is
   // *used*.
-  if (DiagID != diag::warn_unused_comma_left_operand || !isSFINAEContext())
-    DiagIfReachable(Loc, S ? llvm::ArrayRef(S) : std::nullopt,
-                    PDiag(DiagID) << R1 << R2);
+  if (DiagID == diag::warn_unused_comma_left_operand && isSFINAEContext())
+    return;
+
+  // Don't diagnose discarded left of dot in static class member access
+  // because its type is "used" to determine the class to access
+  if (OrigDiagID == diag::warn_discarded_class_member_access)
+    return;
+
+  DiagIfReachable(Loc, S ? llvm::ArrayRef(S) : std::nullopt,
+                  PDiag(DiagID) << R1 << R2);
 }
 
 void Sema::ActOnStartOfCompoundStmt(bool IsStmtExpr) {

clang/test/AST/ast-dump-for-range-lifetime.cpp

@@ -262,19 +262,19 @@ void test7() {
   // CHECK-NEXT:  |           `-MemberExpr {{.*}} '<bound member function type>' .g {{.*}}
   // CHECK-NEXT:  |             `-CXXMemberCallExpr {{.*}} 'A':'P2718R0::A' lvalue
   // CHECK-NEXT:  |               `-MemberExpr {{.*}} '<bound member function type>' .r {{.*}}
-  // CHECK-NEXT:  |                 `-MaterializeTemporaryExpr {{.*}} 'A':'P2718R0::A' xvalue extended by Var {{.*}} '__range1' 'A &&'
+  // CHECK-NEXT:  |                 `-MaterializeTemporaryExpr {{.*}} 'A':'P2718R0::A' lvalue extended by Var {{.*}} '__range1' 'A &&'
   // CHECK-NEXT:  |                   `-CXXBindTemporaryExpr {{.*}} 'A':'P2718R0::A' (CXXTemporary {{.*}})
   // CHECK-NEXT:  |                     `-CXXMemberCallExpr {{.*}} 'A':'P2718R0::A'
   // CHECK-NEXT:  |                       `-MemberExpr {{.*}} '<bound member function type>' .g {{.*}}
   // CHECK-NEXT:  |                         `-CXXMemberCallExpr {{.*}} 'A':'P2718R0::A' lvalue
   // CHECK-NEXT:  |                           `-MemberExpr {{.*}} '<bound member function type>' .r {{.*}}
-  // CHECK-NEXT:  |                             `-MaterializeTemporaryExpr {{.*}} 'A':'P2718R0::A' xvalue extended by Var {{.*}} '__range1' 'A &&'
+  // CHECK-NEXT:  |                             `-MaterializeTemporaryExpr {{.*}} 'A':'P2718R0::A' lvalue extended by Var {{.*}} '__range1' 'A &&'
   // CHECK-NEXT:  |                               `-CXXBindTemporaryExpr {{.*}} 'A':'P2718R0::A' (CXXTemporary {{.*}})
   // CHECK-NEXT:  |                                 `-CXXMemberCallExpr {{.*}} 'A':'P2718R0::A'
   // CHECK-NEXT:  |                                   `-MemberExpr {{.*}} '<bound member function type>' .g {{.*}}
   // CHECK-NEXT:  |                                     `-CXXMemberCallExpr {{.*}} 'A':'P2718R0::A' lvalue
   // CHECK-NEXT:  |                                       `-MemberExpr {{.*}} '<bound member function type>' .r {{.*}}
-  // CHECK-NEXT:  |                                         `-MaterializeTemporaryExpr {{.*}} 'A':'P2718R0::A' xvalue extended by Var {{.*}} '__range1' 'A &&'
+  // CHECK-NEXT:  |                                         `-MaterializeTemporaryExpr {{.*}} 'A':'P2718R0::A' lvalue extended by Var {{.*}} '__range1' 'A &&'
   // CHECK-NEXT:  |                                           `-CXXBindTemporaryExpr {{.*}} 'A':'P2718R0::A' (CXXTemporary {{.*}})
   // CHECK-NEXT:  |                                             `-CallExpr {{.*}} 'A':'P2718R0::A'
   // CHECK-NEXT:  |                                               `-ImplicitCastExpr {{.*}} 'A (*)()' <FunctionToPointerDecay>
@@ -429,19 +429,19 @@ void test13() {
   // CHECK-NEXT:  |           `-MemberExpr {{.*}} '<bound member function type>' .g {{.*}}
   // CHECK-NEXT:  |             `-CXXMemberCallExpr {{.*}} 'A':'P2718R0::A' lvalue
   // CHECK-NEXT:  |               `-MemberExpr {{.*}} '<bound member function type>' .r {{.*}}
-  // CHECK-NEXT:  |                 `-MaterializeTemporaryExpr {{.*}} 'A':'P2718R0::A' xvalue extended by Var {{.*}} '__range1' 'A &&'
+  // CHECK-NEXT:  |                 `-MaterializeTemporaryExpr {{.*}} 'A':'P2718R0::A' lvalue extended by Var {{.*}} '__range1' 'A &&'
   // CHECK-NEXT:  |                   `-CXXBindTemporaryExpr {{.*}} 'A':'P2718R0::A' (CXXTemporary {{.*}})
   // CHECK-NEXT:  |                     `-CXXMemberCallExpr {{.*}} 'A':'P2718R0::A'
   // CHECK-NEXT:  |                       `-MemberExpr {{.*}} '<bound member function type>' .g {{.*}}
   // CHECK-NEXT:  |                         `-CXXMemberCallExpr {{.*}} 'A':'P2718R0::A' lvalue
   // CHECK-NEXT:  |                           `-MemberExpr {{.*}} '<bound member function type>' .r {{.*}}
-  // CHECK-NEXT:  |                             `-MaterializeTemporaryExpr {{.*}} 'A':'P2718R0::A' xvalue extended by Var {{.*}} '__range1' 'A &&'
+  // CHECK-NEXT:  |                             `-MaterializeTemporaryExpr {{.*}} 'A':'P2718R0::A' lvalue extended by Var {{.*}} '__range1' 'A &&'
   // CHECK-NEXT:  |                               `-CXXBindTemporaryExpr {{.*}} 'A':'P2718R0::A' (CXXTemporary {{.*}})
   // CHECK-NEXT:  |                                 `-CXXMemberCallExpr {{.*}} 'A':'P2718R0::A'
   // CHECK-NEXT:  |                                   `-MemberExpr {{.*}} '<bound member function type>' .g {{.*}}
   // CHECK-NEXT:  |                                     `-CXXMemberCallExpr {{.*}} 'A':'P2718R0::A' lvalue
   // CHECK-NEXT:  |                                       `-MemberExpr {{.*}} '<bound member function type>' .r {{.*}}
-  // CHECK-NEXT:  |                                         `-MaterializeTemporaryExpr {{.*}} 'P2718R0::A' xvalue extended by Var {{.*}} '__range1' 'A &&'
+  // CHECK-NEXT:  |                                         `-MaterializeTemporaryExpr {{.*}} 'P2718R0::A' lvalue extended by Var {{.*}} '__range1' 'A &&'
   // CHECK-NEXT:  |                                           `-CXXBindTemporaryExpr {{.*}} 'P2718R0::A' (CXXTemporary {{.*}})
   // CHECK-NEXT:  |                                             `-CallExpr {{.*}} 'P2718R0::A'
   // CHECK-NEXT:  |                                               `-ImplicitCastExpr {{.*}} 'P2718R0::A (*)()' <FunctionToPointerDecay>