[Clang] Implement CWG2813: Class member access with prvalues #95112

MitalAshok · 2024-06-11T13:49:30Z

prvalue.member_fn(expression-list) now will not materialize a temporary for prvalue if member_fn is an explicit object member function, and prvalue will bind directly to the object parameter.

The E1 in E1.static_member is now a discarded-value expression, so if E1 was a call to a [[nodiscard]] function, there will now be a warning. This also affects C++98 with [[gnu::warn_unused_result]] functions.

This should not affect C where TemporaryMaterializationConversion is a no-op.

Closes #100314, fixes #100341

llvmbot · 2024-06-11T13:50:06Z

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clangd

@llvm/pr-subscribers-clang

Author: Mital Ashok (MitalAshok)

Changes

CWG2813

prvalue.member_fn(expression-list) now will not materialize a temporary for prvalue if member_fn is an explicit object member function, and prvalue will bind directly to the object parameter.

The E1 in E1.static_member is now a discarded-value expression, so if E1 was a call to a [[nodiscard]] function, there will now be a warning. This also affects C++98 with [[gnu::warn_unused_result]] functions.

This should not affect C where TemporaryMaterializationConversion is a no-op.

Patch is 22.01 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/95112.diff

10 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+5)
(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3)
(modified) clang/lib/Sema/SemaExprMember.cpp (+52-12)
(modified) clang/lib/Sema/SemaOverload.cpp (+3-8)
(modified) clang/lib/Sema/SemaStmt.cpp (+11-3)
(modified) clang/test/AST/ast-dump-for-range-lifetime.cpp (+6-6)
(modified) clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp (+66-20)
(modified) clang/test/CXX/drs/cwg28xx.cpp (+17)
(modified) clang/test/CodeGenCXX/cxx2b-deducing-this.cpp (-1)
(modified) clang/www/cxx_dr_status.html (+1-1)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cf1ba02cbc4b2..36bf1fdea3602 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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
 ------------------
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 193eae3bc41d6..008bf5fa0ccfc 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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>;
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 3ae1af26d0096..4679fe529ac91 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/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,6 +1222,8 @@ 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,
@@ -1193,6 +1231,8 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
   }
 
   if (VarTemplateDecl *VarTempl = dyn_cast<VarTemplateDecl>(MemberDecl)) {
+    if (MakeDiscardedValue())
+      return ExprError();
     if (!TemplateArgs) {
       diagnoseMissingTemplateArguments(
           SS, /*TemplateKeyword=*/TemplateKWLoc.isValid(), VarTempl, MemberLoc);
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 1b4bcdcb51160..3aa87dae54dd8 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/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())
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 57465d4a77ac2..7effaa943a226 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/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) {
diff --git a/clang/test/AST/ast-dump-for-range-lifetime.cpp b/clang/test/AST/ast-dump-for-range-lifetime.cpp
index 0e92b6990ed50..d66e2c090791e 100644
--- a/clang/test/AST/ast-dump-for-range-lifetime.cpp
+++ b/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>
diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
index e2397c12e2e99..a088c9ab93b0e 100644
--- a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wc++20-extensions %s
-// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify -Wc++17-extensions %s
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify -DEXT -Wc++17-extensions -Wc++20-extensions %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify=expected -Wc++20-extensions %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify=expected,cxx11-17 -Wc++17-extensions %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify=expected,cxx11-17,cxx11 -Wc++17-extensions -Wc++20-extensions %s
 
 struct [[nodiscard]] S {};
 S get_s();
@@ -124,21 +124,67 @@ void usage() {
 }
 }; // namespace p1771
 
-#ifdef EXT
-// expected-warning@5 {{use of the 'nodiscard' attribute is a C++17 extension}}
-// expected-warning@9 {{use of the 'nodiscard' attribute is a C++17 extension}}
-// expected-warning@12 {{use of the 'nodiscard' attribute is a C++17 extension}}
-// expected-warning@13 {{use of the 'nodiscard' attribute is a C++17 extension}}
-// expected-warning@29 {{use of the 'nodiscard' attribute is a C++17 extension}}
-// expected-warning@65 {{use of the 'nodiscard' attribute is a C++20 extension}}
-// expected-warning@67 {{use of the 'nodiscard' attribute is a C++20 extension}}
-// expected-warning@71 {{use of the 'nodiscard' attribute is a C++20 extension}}
-// expected-warning@73 {{use of the 'nodiscard' attribute is a C++20 extension}}
-// expected-warning@74 {{use of the 'nodiscard' attribute is a C++20 extension}}
-// expected-warning@84 {{use of the 'nodiscard' attribute is a C++20 extension}}
-// expected-warning@86 {{use of the 'nodiscard' attribute is a C++17 extension}}
-// expected-warning@87 {{use of the 'nodiscard' attribute is a C++20 extension}}
-// expected-warning@91 {{use of the 'nodiscard' attribute is a C++17 extension}}
-// expected-warning@92 {{use of the 'nodiscard' attribute is a C++20 extension}}
-// expected-warning@95 {{use of the 'nodiscard' attribute is a C++20 extension}}
+namespace discarded_member_access {
+struct X {
+  union {
+    int variant_member;
+  };
+  struct {
+    int anonymous_struct_member;
+  };
+  int data_member;
+  static int static_data_member;
+  enum {
+    unscoped_enum
+  };
+  enum class scoped_enum_t {
+    scoped_enum
+  };
+  using enum scoped_enum_t;
+  // cxx11-17-warning@-1 {{using enum declaration is a C++20 extension}}
+
+  void implicit_object_member_function();
+  static void static_member_function();
+#if __cplusplus >= 202302L
+  void explicit_object_member_function(this X self);
 #endif
+};
+
+[[nodiscard]] X get_X();
+// cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
+void f() {
+  (void) get_X().variant_member;
+  (void) get_X().anonymous_struct_member;
+  (void) get_X().data_member;
+  (void) get_X().static_data_member;
+  // expected-warning@-1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+  (void) get_X().unscoped_enum;
+  // expected-warning@-1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+  (void) get_X().scoped_enum;
+  // expected-warning@-1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+  (void) get_X().implicit_object_member_function();
+  (void) get_X().static_member_function();
+  // expected-warning@-1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+#if __cplusplus >= 202302L
+  (void) get_X().explicit_object_member_function();
+#endif
+}
+} // namespace discarded_member_access
+
+
+// cxx11-warning@5 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// cxx11-warning@9 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// cxx11-warning@12 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// cxx11-warning@13 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// cxx11-warning@29 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// cxx11-warning@65 {{use of the 'nodiscard' attribute is a C++20 extension}}
+// cxx11-warning@67 {{use of the 'nodiscard' attribute is a C++20 extension}}
+// cxx11-warning@71 {{use of the 'nodiscard' attribute is a C++20 extension}}
+// cxx11-warning@73 {{use of the 'nodiscard' attribute is a C++20 extension}}
+// cxx11-warning@74 {{use of the 'nodiscard' attribute is a C++20 extension}}
+// cxx11-warning@84 {{use of the 'nodiscard' attribute is a C++20 extension}}
+// cxx11-warning@86 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// cxx11-warning@87 {{use of the 'nodiscard' attribute is a C++20 extension}}
+// cxx11-warning@91 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// cxx11-warning@92 {{use of the 'nodiscard' attribute is a C++20 extension}}
+// cxx11-warning@...
[truncated]

Sirraide · 2024-06-13T19:30:22Z

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">,


Suggested change

"left operand of dot in this class member access is discarded and has no effect">,

"discarding left operand of '.' in %select{access to static data member|access to enumerator|call to static member function}0">,

(and maybe more cases if I missed any)

From what I can tell, the call to the MakeDiscardedValue() lambda that you’ve added could take the integer for the %select here as a parameter.

clang/lib/Sema/SemaExprMember.cpp

clang/lib/Sema/SemaStmt.cpp

clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp

Create Sema::DiagnoseDiscardedNodiscard Remove warn_discarded_class_member_access and call Sema::DiagnoseDiscardedNodiscard instead Remove MakeGLValue from the MSPropertyRefExpr path Fix [[nodiscard]] test in test/CXX Add test for [[nodiscard]] and explicit object member functions in test/SemaCXX

Sirraide

A few more minor things, but LGTM otherwise.

clang/lib/Sema/SemaStmt.cpp

Sirraide · 2024-06-17T09:26:19Z

clang/lib/Sema/SemaStmt.cpp

+void DiagnoseUnused(Sema &S, const Expr *E, std::optional<unsigned> DiagID) {
+  bool NoDiscardOnly = !DiagID.has_value();


A comment explaining the relationship between the DiagID parameter and NoDiscardOnly here would be nice so anyone looking at calls to this knows what’s going on w/o having to go through the entire function.

clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp

github-actions · 2024-06-17T19:46:03Z

✅ With the latest revision this PR passed the C/C++ code formatter.

cor3ntin · 2024-07-26T08:55:41Z

clang/include/clang/Sema/Sema.h

+  /// DiagnoseDiscardedNodiscard - Given an expression that is semantically
+  /// a discarded-value expression, diagnose if any [[nodiscard]] value
+  /// has been discarded
+  void DiagnoseDiscardedNodiscard(const Expr *E);


Can we find a better name for that function?

cor3ntin

General direction looks good

cor3ntin · 2024-07-26T09:14:59Z

clang/lib/Sema/SemaStmt.cpp

+  // When called from Sema::DiagnoseUnusedExprResult, DiagID is a diagnostic for
+  // where this expression is not used. When called from
+  // Sema::DiagnoseDiscardedNodiscard, DiagID is std::nullopt and this function
+  // will only diagnose [[nodiscard]], [[gnu::warn_unused_result]] and similar


Suggested change

// When called from Sema::DiagnoseUnusedExprResult, DiagID is a diagnostic for

// where this expression is not used. When called from

// Sema::DiagnoseDiscardedNodiscard, DiagID is std::nullopt and this function

// will only diagnose [[nodiscard]], [[gnu::warn_unused_result]] and similar

// Diagnoses unused expressions that call functions marked [[nodiscard]], [[gnu::warn_unused_result]] and similar. Additionally, a DiagID can be provided to emit a warning in additional contexts (such as for an unused LHS of a comma expression)

TBH, warn_unused_comma_left_operand seem to be the only scenario at a glance so maybe we would be better off introduce an enum for that 3rd parameter

@MitalAshok ping :)

@MitalAshok ping again :)

There are not a lot of outstanding known issues with deducing this (besides llvm#95112), so it seems reasonable to claim full support. Fixes llvm#82780

h-vetinari · 2024-12-16T10:10:31Z

clang/www/cxx_dr_status.html

@@ -16694,7 +16694,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
    <td><a href="https://cplusplus.github.io/CWG/issues/2813.html">2813</a></td>
    <td>DR</td>
    <td>Class member access with prvalues</td>
-    <td class="unknown" align="center">Unknown</td>
+    <td class="unreleased" align="center">Clang 20</td>


Given the lack of activity here, this looks to be in danger of slipping - branch is in about a month (going by the llvm 18 schedule)

Yeah, if @MitalAshok can't push that through the finish line, Maybe one of us need to take over

@MitalAshok

This is a rebase of #95112 with my own feedback apply as @MitalAshok has been inactive for a while. It's fairly important this makes clang 20 as it is a blocker for #107451 --- [CWG2813](https://cplusplus.github.io/CWG/issues/2813.html) prvalue.member_fn(expression-list) now will not materialize a temporary for prvalue if member_fn is an explicit object member function, and prvalue will bind directly to the object parameter. The E1 in E1.static_member is now a discarded-value expression, so if E1 was a call to a [[nodiscard]] function, there will now be a warning. This also affects C++98 with [[gnu::warn_unused_result]] functions. This should not affect C where TemporaryMaterializationConversion is a no-op. Closes #100314 Fixes #100341 --------- Co-authored-by: Mital Ashok <[email protected]>

There are not a lot of outstanding known issues with deducing this (besides #95112), so it seems reasonable to claim full support. Fixes #82780

@MitalAshok

…0223) This is a rebase of llvm#95112 with my own feedback apply as @MitalAshok has been inactive for a while. It's fairly important this makes clang 20 as it is a blocker for llvm#107451 Change-Id: I2261810f7c8d7dd8b3e3412c0814a528d7c7b91c --- [CWG2813](https://cplusplus.github.io/CWG/issues/2813.html) prvalue.member_fn(expression-list) now will not materialize a temporary for prvalue if member_fn is an explicit object member function, and prvalue will bind directly to the object parameter. The E1 in E1.static_member is now a discarded-value expression, so if E1 was a call to a [[nodiscard]] function, there will now be a warning. This also affects C++98 with [[gnu::warn_unused_result]] functions. This should not affect C where TemporaryMaterializationConversion is a no-op. Closes llvm#100314 Fixes llvm#100341 --------- Co-authored-by: Mital Ashok <[email protected]> (cherry picked from commit db93ef1) Change-Id: Ic277d16bc8611b9d383cb890da3eda0ef1646555

[Clang] Implement CWG2813

e53dfbc

MitalAshok requested a review from Endilll as a code owner June 11, 2024 13:49

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 11, 2024

fix clangd AST test

edac756

llvmbot added clang-tools-extra clangd labels Jun 11, 2024

Run [[nodiscard]] test in C++23

a353728

cor3ntin requested review from mizvekov, EricWF, AaronBallman, cor3ntin and Sirraide June 13, 2024 08:19

Sirraide reviewed Jun 13, 2024

View reviewed changes

Sirraide approved these changes Jun 17, 2024

View reviewed changes

llvmbot added the clang:codegen IR generation bugs: mangling, exceptions, etc. label Jun 17, 2024

MitalAshok added 2 commits June 17, 2024 20:48

Merge branch 'main' into prvalue-explicit-object-member-function-call

3fe511a

anonymous namespace -> static

9108fd0

MitalAshok force-pushed the prvalue-explicit-object-member-function-call branch from 37f2448 to 9108fd0 Compare June 17, 2024 19:48

MitalAshok added 4 commits June 29, 2024 12:16

Merge branch 'main' into prvalue-explicit-object-member-function-call

1ff7db1

Update cwg28xx.cpp

087a9f0

Merge branch 'main' into prvalue-explicit-object-member-function-call

e839e95

Fix test failure

8e72680

cor3ntin mentioned this pull request Jul 24, 2024

CWG2813: extra move in calling of explicit object member function #100314

Closed

MitalAshok added 3 commits July 24, 2024 17:52

Merge branch 'main' into prvalue-explicit-object-member-function-call

ef71fb7

Add example from llvm#100341

95440d6

Retarget clang 20

78a3115

MitalAshok force-pushed the prvalue-explicit-object-member-function-call branch from 0af31db to 78a3115 Compare July 24, 2024 17:18

MitalAshok mentioned this pull request Jul 24, 2024

[Clang] Deducing this does not work correctly with move-only types. #100341

Closed

cor3ntin reviewed Jul 26, 2024

View reviewed changes

cor3ntin mentioned this pull request Sep 5, 2024

[Clang] Set __cpp_explicit_this_parameter #107451

Merged

h-vetinari reviewed Dec 16, 2024

View reviewed changes

cor3ntin mentioned this pull request Dec 17, 2024

[Clang] Implement CWG2813: Class member access with prvalues #120223

Merged

cor3ntin closed this Dec 18, 2024

cor3ntin added a commit that referenced this pull request Dec 18, 2024

[Clang] Set __cpp_explicit_this_parameter (#107451)

72e58e0

There are not a lot of outstanding known issues with deducing this (besides #95112), so it seems reasonable to claim full support. Fixes #82780

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang] Implement CWG2813: Class member access with prvalues #95112

[Clang] Implement CWG2813: Class member access with prvalues #95112

MitalAshok commented Jun 11, 2024 •

edited

Loading

llvmbot commented Jun 11, 2024 •

edited

Loading

Sirraide Jun 13, 2024

Sirraide Jun 13, 2024

Sirraide left a comment

Sirraide Jun 17, 2024

github-actions bot commented Jun 17, 2024 •

edited

Loading

cor3ntin Jul 26, 2024

cor3ntin left a comment

cor3ntin Jul 26, 2024

cor3ntin Jul 26, 2024

cor3ntin Sep 5, 2024

cor3ntin Sep 17, 2024

h-vetinari Dec 16, 2024

cor3ntin Dec 16, 2024

	"left operand of dot in this class member access is discarded and has no effect">,
	"discarding left operand of '.' in %select{access to static data member\|access to enumerator\|call to static member function}0">,

		void DiagnoseUnused(Sema &S, const Expr *E, std::optional<unsigned> DiagID) {
		bool NoDiscardOnly = !DiagID.has_value();

[Clang] Implement CWG2813: Class member access with prvalues #95112

[Clang] Implement CWG2813: Class member access with prvalues #95112

Conversation

MitalAshok commented Jun 11, 2024 • edited Loading

llvmbot commented Jun 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sirraide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

cor3ntin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MitalAshok commented Jun 11, 2024 •

edited

Loading

llvmbot commented Jun 11, 2024 •

edited

Loading

github-actions bot commented Jun 17, 2024 •

edited

Loading