diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -9151,15 +9151,22 @@ "before C++20">, InGroup, DefaultIgnore; def err_defaulted_comparison_template : Error< "comparison operator template cannot be defaulted">; -def err_defaulted_comparison_out_of_class : Error< - "%sub{select_defaulted_comparison_kind}0 can only be defaulted in a class " - "definition">; +def err_defaulted_comparison_num_args : Error< + "%select{non-member|member}0 %sub{select_defaulted_comparison_kind}1" + " comparison operator must have %select{2|1}0 parameters">; def err_defaulted_comparison_param : Error< "invalid parameter type for defaulted %sub{select_defaulted_comparison_kind}0" "; found %1, expected %2%select{| or %4}3">; +def err_defaulted_comparison_param_unknown : Error< + "invalid parameter type for non-member defaulted" + " %sub{select_defaulted_comparison_kind}0" + "; found %1, expected class or reference to a constant class">; def err_defaulted_comparison_param_mismatch : Error< "parameters for defaulted %sub{select_defaulted_comparison_kind}0 " "must have the same type%diff{ (found $ vs $)|}1,2">; +def err_defaulted_comparison_not_friend : Error< + "%sub{select_defaulted_comparison_kind}0 is not a friend of" + " %select{|incomplete class }1%2">; def err_defaulted_comparison_non_const : Error< "defaulted member %sub{select_defaulted_comparison_kind}0 must be " "const-qualified">; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -8435,9 +8435,6 @@ DefaultedComparisonKind DCK) { assert(DCK != DefaultedComparisonKind::None && "not a defaulted comparison"); - CXXRecordDecl *RD = dyn_cast(FD->getLexicalDeclContext()); - assert(RD && "defaulted comparison is not defaulted in a class"); - // Perform any unqualified lookups we're going to need to default this // function. if (S) { @@ -8455,43 +8452,17 @@ // const C&, or // -- a friend of C having two parameters of type const C& or two // parameters of type C. - QualType ExpectedParmType1 = Context.getRecordType(RD); - QualType ExpectedParmType2 = - Context.getLValueReferenceType(ExpectedParmType1.withConst()); - if (isa(FD)) - ExpectedParmType1 = ExpectedParmType2; - for (const ParmVarDecl *Param : FD->parameters()) { - if (!Param->getType()->isDependentType() && - !Context.hasSameType(Param->getType(), ExpectedParmType1) && - !Context.hasSameType(Param->getType(), ExpectedParmType2)) { - // Don't diagnose an implicit 'operator=='; we will have diagnosed the - // corresponding defaulted 'operator<=>' already. - if (!FD->isImplicit()) { - Diag(FD->getLocation(), diag::err_defaulted_comparison_param) - << (int)DCK << Param->getType() << ExpectedParmType1 - << !isa(FD) - << ExpectedParmType2 << Param->getSourceRange(); - } - return true; - } - } - if (FD->getNumParams() == 2 && - !Context.hasSameType(FD->getParamDecl(0)->getType(), - FD->getParamDecl(1)->getType())) { - if (!FD->isImplicit()) { - Diag(FD->getLocation(), diag::err_defaulted_comparison_param_mismatch) - << (int)DCK - << FD->getParamDecl(0)->getType() - << FD->getParamDecl(0)->getSourceRange() - << FD->getParamDecl(1)->getType() - << FD->getParamDecl(1)->getSourceRange(); - } - return true; - } - // ... non-static const member ... - if (auto *MD = dyn_cast(FD)) { + CXXRecordDecl *RD = dyn_cast(FD->getLexicalDeclContext()); + bool IsMethod = isa(FD); + if (IsMethod) { + auto *MD = cast(FD); assert(!MD->isStatic() && "comparison function cannot be a static member"); + + // If we're out-of-class, this is the class we're comparing. + if (!RD) + RD = MD->getParent(); + if (!MD->isConst()) { SourceLocation InsertLoc; if (FunctionTypeLoc Loc = MD->getFunctionTypeLoc()) @@ -8500,7 +8471,7 @@ // corresponding defaulted 'operator<=>' already. if (!MD->isImplicit()) { Diag(MD->getLocation(), diag::err_defaulted_comparison_non_const) - << (int)DCK << FixItHint::CreateInsertion(InsertLoc, " const"); + << (int)DCK << FixItHint::CreateInsertion(InsertLoc, " const"); } // Add the 'const' to the type to recover. @@ -8510,9 +8481,100 @@ MD->setType(Context.getFunctionType(FPT->getReturnType(), FPT->getParamTypes(), EPI)); } - } else { - // A non-member function declared in a class must be a friend. + } + + if (FD->getNumParams() != (IsMethod ? 1 : 2)) { + // Let's not worry about using a variadic template pack here -- who would do + // such a thing? + Diag(FD->getLocation(), diag::err_defaulted_comparison_num_args) + << int(IsMethod) << int(DCK); + return true; + } + + const ParmVarDecl *KnownParm = nullptr; + for (const ParmVarDecl *Param : FD->parameters()) { + QualType ParmTy = Param->getType(); + if (ParmTy->isDependentType()) + continue; + if (!KnownParm) { + auto CTy = ParmTy; + // Is it `T const &`? + bool Ok = !IsMethod; + QualType ExpectedTy; + if (RD) + ExpectedTy = Context.getRecordType(RD); + if (auto *Ref = CTy->getAs()) { + CTy = Ref->getPointeeType(); + if (RD) + ExpectedTy.addConst(); + Ok = true; + } + + // Is T a class? + if (!Ok) { + } else if (RD) { + if (!RD->isDependentType() && !Context.hasSameType(CTy, ExpectedTy)) + Ok = false; + } else if (auto *CRD = CTy->getAsRecordDecl()) { + RD = cast(CRD); + } else { + Ok = false; + } + + if (Ok) { + KnownParm = Param; + } else { + // Don't diagnose an implicit 'operator=='; we will have diagnosed the + // corresponding defaulted 'operator<=>' already. + if (!FD->isImplicit()) { + if (RD) { + QualType PlainTy = Context.getRecordType(RD); + QualType RefTy = + Context.getLValueReferenceType(PlainTy.withConst()); + if (IsMethod) + PlainTy = QualType(); + Diag(FD->getLocation(), diag::err_defaulted_comparison_param) + << int(DCK) << ParmTy << RefTy << int(!IsMethod) << PlainTy + << Param->getSourceRange(); + } else { + assert(!IsMethod && "should know expected type for method"); + Diag(FD->getLocation(), + diag::err_defaulted_comparison_param_unknown) + << int(DCK) << ParmTy << Param->getSourceRange(); + } + } + return true; + } + } else if (!Context.hasSameType(KnownParm->getType(), ParmTy)) { + Diag(FD->getLocation(), diag::err_defaulted_comparison_param_mismatch) + << int(DCK) << KnownParm->getType() << KnownParm->getSourceRange() + << ParmTy << Param->getSourceRange(); + return true; + } + } + + assert(RD && "must have determined class"); + if (IsMethod) { + } else if (isa(FD->getLexicalDeclContext())) { + // In-class, must be a friend decl. assert(FD->getFriendObjectKind() && "expected a friend declaration"); + } else { + // Out of class, require the defaulted comparison to be a friend (of a + // complete type). + if (RequireCompleteType(FD->getLocation(), Context.getRecordType(RD), + diag::err_defaulted_comparison_not_friend, int(DCK), + int(1))) + return true; + + if (llvm::find_if(RD->friends(), [&](const FriendDecl *F) { + return FD->getCanonicalDecl() == + F->getFriendDecl()->getCanonicalDecl(); + }) == RD->friends().end()) { + Diag(FD->getLocation(), diag::err_defaulted_comparison_not_friend) + << int(DCK) << int(0) << RD; + Diag(RD->getCanonicalDecl()->getLocation(), diag::note_declared_at); + return true; + } } // C++2a [class.eq]p1, [class.rel]p1: @@ -8670,7 +8732,10 @@ { // Build and set up the function body. - CXXRecordDecl *RD = cast(FD->getLexicalParent()); + // The first parameter has type maybe-ref-to maybe-const T, use that to get + // the type of the class being compared. + auto PT = FD->getParamDecl(0)->getType(); + CXXRecordDecl *RD = PT.getNonReferenceType()->getAsCXXRecordDecl(); SourceLocation BodyLoc = FD->getEndLoc().isValid() ? FD->getEndLoc() : FD->getLocation(); StmtResult Body = @@ -17180,13 +17245,6 @@ return; } - if (DefKind.isComparison() && - !isa(FD->getLexicalDeclContext())) { - Diag(FD->getLocation(), diag::err_defaulted_comparison_out_of_class) - << (int)DefKind.asComparison(); - return; - } - // Issue compatibility warning. We already warned if the operator is // 'operator<=>' when parsing the '<=>' token. if (DefKind.isComparison() && @@ -17208,31 +17266,37 @@ // that we've marked it as defaulted. FD->setWillHaveBody(false); - // If this definition appears within the record, do the checking when - // the record is complete. This is always the case for a defaulted - // comparison. - if (DefKind.isComparison()) + // If this is a comparison's defaulted definition within the record, do + // the checking when the record is complete. + if (DefKind.isComparison() && isa(FD->getLexicalDeclContext())) return; - auto *MD = cast(FD); - const FunctionDecl *Primary = FD; - if (const FunctionDecl *Pattern = FD->getTemplateInstantiationPattern()) - // Ask the template instantiation pattern that actually had the - // '= default' on it. - Primary = Pattern; - - // If the method was defaulted on its first declaration, we will have + // If this member fn was defaulted on its first declaration, we will have // already performed the checking in CheckCompletedCXXClass. Such a // declaration doesn't trigger an implicit definition. - if (Primary->getCanonicalDecl()->isDefaulted()) - return; + if (isa(FD)) { + const FunctionDecl *Primary = FD; + if (const FunctionDecl *Pattern = FD->getTemplateInstantiationPattern()) + // Ask the template instantiation pattern that actually had the + // '= default' on it. + Primary = Pattern; + if (Primary->getCanonicalDecl()->isDefaulted()) + return; + } - // FIXME: Once we support defining comparisons out of class, check for a - // defaulted comparison here. - if (CheckExplicitlyDefaultedSpecialMember(MD, DefKind.asSpecialMember())) - MD->setInvalidDecl(); - else - DefineDefaultedFunction(*this, MD, DefaultLoc); + if (DefKind.isComparison()) { + if (CheckExplicitlyDefaultedComparison(nullptr, FD, DefKind.asComparison())) + FD->setInvalidDecl(); + else + DefineDefaultedComparison(DefaultLoc, FD, DefKind.asComparison()); + } else { + auto *MD = cast(FD); + + if (CheckExplicitlyDefaultedSpecialMember(MD, DefKind.asSpecialMember())) + MD->setInvalidDecl(); + else + DefineDefaultedFunction(*this, MD, DefaultLoc); + } } static void SearchForReturnInStmt(Sema &Self, Stmt *S) { diff --git a/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp b/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp --- a/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp +++ b/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp @@ -1,15 +1,13 @@ // RUN: %clang_cc1 -std=c++2a -verify %s struct B {}; -bool operator==(const B&, const B&) = default; // expected-error {{equality comparison operator can only be defaulted in a class definition}} expected-note {{candidate}} -bool operator<=>(const B&, const B&) = default; // expected-error {{three-way comparison operator can only be defaulted in a class definition}} template bool operator<(const B&, const B&) = default; // expected-error {{comparison operator template cannot be defaulted}} struct A { friend bool operator==(const A&, const A&) = default; - friend bool operator!=(const A&, const B&) = default; // expected-error {{invalid parameter type for defaulted equality comparison operator; found 'const B &', expected 'A' or 'const A &'}} + friend bool operator!=(const A&, const B&) = default; // expected-error {{parameters for defaulted equality comparison operator must have the same type (found 'const A &' vs 'const B &')}} friend bool operator!=(const B&, const B&) = default; // expected-error {{invalid parameter type for defaulted equality comparison}} friend bool operator<(const A&, const A&); friend bool operator<(const B&, const B&) = default; // expected-error {{invalid parameter type for defaulted relational comparison}} @@ -29,11 +27,6 @@ bool operator==(const A&) const = default; // expected-error {{comparison operator template cannot be defaulted}} }; -// FIXME: The wording is not clear as to whether these are valid, but the -// intention is that they are not. -bool operator<(const A&, const A&) = default; // expected-error {{relational comparison operator can only be defaulted in a class definition}} -bool A::operator<(const A&) const = default; // expected-error {{can only be defaulted in a class definition}} - template struct Dependent { using U = typename T::type; bool operator==(U) const = default; // expected-error {{found 'Dependent::U'}} @@ -132,3 +125,41 @@ friend bool operator==(const B&, const B&) = default; // expected-warning {{deleted}} }; } + +namespace p2085 { +// out-of-class defaulting + +struct S1 { + bool operator==(S1 const &) const; +}; + +bool S1::operator==(S1 const &) const = default; + +bool F1(S1 &s) { + return s != s; +} + +struct S2 { + friend bool operator==(S2 const &, S2 const &); +}; + +bool operator==(S2 const &, S2 const &) = default; +bool F2(S2 &s) { + return s != s; +} + +struct S3 {}; // expected-note{{here}} +bool operator==(S3 const &, S3 const &) = default; // expected-error{{not a friend}} + +struct S4; // expected-note{{forward declaration}} +bool operator==(S4 const &, S4 const &) = default; // expected-error{{not a friend}} + +struct S5; // expected-note 3{{forward declaration}} +bool operator==(S5, S5) = default; // expected-error{{not a friend}} expected-error 2{{has incomplete type}} + +enum e {}; +bool operator==(e, int) = default; // expected-error{{expected class or reference to a constant class}} + +bool operator==(e *, int *) = default; // expected-error{{must have at least one}} + +} // namespace p2085 diff --git a/clang/test/CodeGenCXX/p2085.cpp b/clang/test/CodeGenCXX/p2085.cpp new file mode 100644 --- /dev/null +++ b/clang/test/CodeGenCXX/p2085.cpp @@ -0,0 +1,38 @@ +// RUN: %clang_cc1 --std=c++20 %s -emit-llvm -o - -triple x86_64-linux | FileCheck %s + +namespace std { +struct strong_ordering { + int n; + constexpr operator int() const { return n; } + static const strong_ordering equal, greater, less; +}; +constexpr inline strong_ordering strong_ordering::equal = {0}; +constexpr inline strong_ordering strong_ordering::greater = {1}; +constexpr inline strong_ordering strong_ordering::less = {-1}; +} // namespace std + +struct Space { + int i, j; + + std::strong_ordering operator<=>(Space const &other) const; + bool operator==(Space const &other) const; +}; + +// Make sure these cause emission +std::strong_ordering Space::operator<=>(Space const &other) const = default; +// CHECK-LABEL: define{{.*}} @_ZNK5SpacessERKS_ +bool Space::operator==(Space const &) const = default; +// CHECK-LABEL: define{{.*}} @_ZNK5SpaceeqERKS_ + +struct Water { + int i, j; + + std::strong_ordering operator<=>(Water const &other) const; + bool operator==(Water const &other) const; +}; + +// Make sure these do not cause emission +inline std::strong_ordering Water::operator<=>(Water const &other) const = default; +// CHECK-NOT: define{{.*}} @_ZNK5WaterssERKS_ +inline bool Water::operator==(Water const &) const = default; +// CHECK-NOT: define{{.*}} @_ZNK5WatereqERKS_ diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -1006,7 +1006,7 @@ P2085R0 - No + Clang 14 Access checking on specializations