llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Mital Ashok (MitalAshok) <details> <summary>Changes</summary> [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. --- 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 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] `````````` </details> https://github.com/llvm/llvm-project/pull/95112 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
