llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (offsetof) <details> <summary>Changes</summary> * Move validation of operator function parameters into a new routine, `Sema::CheckOverloadedOperatorParams`. * Treat operator function template specializations failing these checks as a substitution failure. * Allow binary operator function templates to have one parameter if that parameter is a pack. * Diagnose some operator function templates that could never produce a valid specialization based on their parameter types. * Diagnose operator functions with an explicit object parameter and no parameters of class or enumeration type. Fixes #<!-- -->49197 Fixes #<!-- -->126855 Fixes #<!-- -->128902 --- Patch is 35.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/131777.diff 9 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+8) - (modified) clang/include/clang/Sema/Sema.h (+8) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+114-103) - (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+23) - (modified) clang/test/Parser/cxx20-coroutines.cpp (+1-1) - (modified) clang/test/SemaCXX/cxx2a-three-way-comparison.cpp (+1-1) - (modified) clang/test/SemaCXX/overloaded-operator-decl.cpp (+14-12) - (modified) clang/test/SemaCXX/overloaded-operator.cpp (+17-12) - (modified) clang/test/SemaTemplate/operator-template.cpp (+173-13) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d9f1c95533c9c..d7e6dcfe5d10e 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -267,6 +267,7 @@ Improvements to Clang's diagnostics - Improve the diagnostics for chained comparisons to report actual expressions and operators (#GH129069). - Improve the diagnostics for shadows template parameter to report correct location (#GH129060). +- Some operator function templates that could never produce a valid specialization are now diagnosed at definition time. Improvements to Clang's time-trace ---------------------------------- @@ -330,6 +331,13 @@ Bug Fixes to C++ Support - Fixed an assertion failure affecting code that uses C++23 "deducing this". (#GH130272) - Clang now properly instantiates destructors for initialized members within non-delegating constructors. (#GH93251) - Correctly diagnoses if unresolved using declarations shadows template paramters (#GH129411) +- Invalid operator function signatures generated from templates during + overload resolution are now eliminated from the candidate set without making + the program ill-formed. (#GH49197) +- Binary operator function templates with a single parameter are no longer + rejected when that parameter is a pack. +- Fixed operator functions without a class or enumeration parameter not being + rejected when an explicit object parameter is present. Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index fc3936d649320..923ff3edec9a5 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5863,6 +5863,14 @@ class Sema final : public SemaBase { // C++ Overloaded Operators [C++ 13.5] // + /// CheckOverloadedOperatorParams - Check whether the parameter-type-list + /// of an overloaded operator is valid (has the correct number of + /// parameters for the operator, at least one parameter is of a class or + /// enumeration type, and, for the postfix increment/decrement operators, + /// the last parameter is of type int). If so, returns false; otherwise, + /// emits appropriate diagnostics and returns true. + bool CheckOverloadedOperatorParams(FunctionDecl *FnDecl); + /// CheckOverloadedOperatorDeclaration - Check whether the declaration /// of this overloaded operator is well-formed. If so, returns false; /// otherwise, emits appropriate diagnostics and returns true. diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 20533961a2217..69e66499b016c 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -27,6 +27,7 @@ #include "clang/AST/TypeLoc.h" #include "clang/AST/TypeOrdering.h" #include "clang/Basic/AttributeCommonInfo.h" +#include "clang/Basic/OperatorKinds.h" #include "clang/Basic/PartialDiagnostic.h" #include "clang/Basic/Specifiers.h" #include "clang/Basic/TargetInfo.h" @@ -47,6 +48,7 @@ #include "clang/Sema/SemaOpenMP.h" #include "clang/Sema/Template.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/Bitset.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/StringExtras.h" @@ -16404,6 +16406,94 @@ CheckOperatorDeleteDeclaration(Sema &SemaRef, FunctionDecl *FnDecl) { return false; } +bool Sema::CheckOverloadedOperatorParams(FunctionDecl *FnDecl) { + assert(FnDecl && FnDecl->isOverloadedOperator() && + "Expected an overloaded operator declaration"); + + auto Op = FnDecl->getOverloadedOperator(); + bool IsImplicitObjectMemFn = isa<CXXMethodDecl>(FnDecl) && + !FnDecl->hasCXXExplicitFunctionObjectParameter(); + auto Params = FnDecl->parameters(); + auto NumParams = Params.size() + IsImplicitObjectMemFn; + + // C++2c [over.oper.general] p10: + // Operator functions cannot have more or fewer parameters than the + // number required for the corresponding operator, as described in the + // rest of [over.oper]. + static constexpr llvm::Bitset<NUM_OVERLOADED_OPERATORS> UnaryOps{ +#define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary, MemberOnly) \ + Unary ? OO_##Name : OO_None, +#include "clang/Basic/OperatorKinds.def" + }; + static constexpr llvm::Bitset<NUM_OVERLOADED_OPERATORS> BinaryOps{ +#define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary, MemberOnly) \ + Binary ? OO_##Name : OO_None, +#include "clang/Basic/OperatorKinds.def" + }; + + if (Op == OO_Subscript) { + if (NumParams != 2) { + // 0 = no parameters, 2 = more than one parameter + int ErrorKind = NumParams == 1 ? 0 : 2; + Diag(FnDecl->getLocation(), LangOpts.CPlusPlus23 + ? diag::ext_subscript_overload + : diag::error_subscript_overload) + << FnDecl->getDeclName() << ErrorKind; + if (!LangOpts.CPlusPlus23) + return true; + } + } else if (Op != OO_Call) { + bool NumParamsIsValid = false; + if (NumParams == 1) + NumParamsIsValid = UnaryOps[Op] || (!IsImplicitObjectMemFn && + Params[0]->isParameterPack()); + else if (NumParams == 2) + NumParamsIsValid = BinaryOps[Op]; + + if (!NumParamsIsValid) { + // 0 = unary, 1 = binary, 2 = unary or binary + int ErrorKind = (BinaryOps[Op] << 1 | UnaryOps[Op]) - 1; + return Diag(FnDecl->getLocation(), diag::err_operator_overload_must_be) + << FnDecl->getDeclName() << NumParams << ErrorKind; + } + } + + // The second parameter of operator++ and operator--, if present, + // must be of type 'int' (C++2c [over.inc] p1). + if ((Op == OO_PlusPlus || Op == OO_MinusMinus) && NumParams == 2) { + ParmVarDecl *SecondParam = Params[!IsImplicitObjectMemFn]; + QualType Type = SecondParam->getType(); + if (!(Type->isSpecificBuiltinType(BuiltinType::Int) || + Type->isDependentType())) + return Diag(SecondParam->getLocation(), + diag::err_operator_overload_post_incdec_must_be_int) + << Type << (Op == OO_MinusMinus); + + // Ignore the 'int' parameter for subsequent checks. + Params = Params.drop_back(); + } + + // C++2c [over.oper.general] p7: + // An operator function shall have at least one function parameter + // or implicit object parameter whose type is a class, a reference + // to a class, an enumeration, or a reference to an enumeration. + bool HasClassOrEnumParam = + IsImplicitObjectMemFn || llvm::any_of(Params, [](ParmVarDecl *Param) { + QualType Type = + Param->getType().getNonPackExpansionType().getNonReferenceType(); + return Type->isDependentType() + ? !(Type->isPointerType() || Type->isMemberPointerType() || + Type->isFunctionType() || Type->isArrayType()) + : Type->isRecordType() || Type->isEnumeralType(); + }); + if (!HasClassOrEnumParam) + return Diag(FnDecl->getLocation(), + diag::err_operator_overload_needs_class_or_enum) + << FnDecl->getDeclName(); + + return false; +} + bool Sema::CheckOverloadedOperatorDeclaration(FunctionDecl *FnDecl) { assert(FnDecl && FnDecl->isOverloadedOperator() && "Expected an overloaded operator declaration"); @@ -16422,40 +16512,19 @@ bool Sema::CheckOverloadedOperatorDeclaration(FunctionDecl *FnDecl) { if (Op == OO_New || Op == OO_Array_New) return CheckOperatorNewDeclaration(*this, FnDecl); - // C++ [over.oper]p7: - // An operator function shall either be a member function or - // be a non-member function and have at least one parameter - // whose type is a class, a reference to a class, an enumeration, - // or a reference to an enumeration. - // Note: Before C++23, a member function could not be static. The only member - // function allowed to be static is the call operator function. - if (CXXMethodDecl *MethodDecl = dyn_cast<CXXMethodDecl>(FnDecl)) { - if (MethodDecl->isStatic()) { - if (Op == OO_Call || Op == OO_Subscript) - Diag(FnDecl->getLocation(), - (LangOpts.CPlusPlus23 - ? diag::warn_cxx20_compat_operator_overload_static - : diag::ext_operator_overload_static)) - << FnDecl; - else - return Diag(FnDecl->getLocation(), diag::err_operator_overload_static) - << FnDecl; - } - } else { - bool ClassOrEnumParam = false; - for (auto *Param : FnDecl->parameters()) { - QualType ParamType = Param->getType().getNonReferenceType(); - if (ParamType->isDependentType() || ParamType->isRecordType() || - ParamType->isEnumeralType()) { - ClassOrEnumParam = true; - break; - } - } - - if (!ClassOrEnumParam) - return Diag(FnDecl->getLocation(), - diag::err_operator_overload_needs_class_or_enum) - << FnDecl->getDeclName(); + // Only function call and subscript operators are allowed to be + // static member functions, and only since C++23. + if (auto *MethodDecl = dyn_cast<CXXMethodDecl>(FnDecl); + MethodDecl && MethodDecl->isStatic()) { + if (Op == OO_Call || Op == OO_Subscript) + Diag(FnDecl->getLocation(), + (LangOpts.CPlusPlus23 + ? diag::warn_cxx20_compat_operator_overload_static + : diag::ext_operator_overload_static)) + << FnDecl; + else + return Diag(FnDecl->getLocation(), diag::err_operator_overload_static) + << FnDecl; } // C++ [over.oper]p8: @@ -16488,52 +16557,6 @@ bool Sema::CheckOverloadedOperatorDeclaration(FunctionDecl *FnDecl) { } } - static const bool OperatorUses[NUM_OVERLOADED_OPERATORS][3] = { - { false, false, false } -#define OVERLOADED_OPERATOR(Name,Spelling,Token,Unary,Binary,MemberOnly) \ - , { Unary, Binary, MemberOnly } -#include "clang/Basic/OperatorKinds.def" - }; - - bool CanBeUnaryOperator = OperatorUses[Op][0]; - bool CanBeBinaryOperator = OperatorUses[Op][1]; - bool MustBeMemberOperator = OperatorUses[Op][2]; - - // C++ [over.oper]p8: - // [...] Operator functions cannot have more or fewer parameters - // than the number required for the corresponding operator, as - // described in the rest of this subclause. - unsigned NumParams = FnDecl->getNumParams() + - (isa<CXXMethodDecl>(FnDecl) && - !FnDecl->hasCXXExplicitFunctionObjectParameter() - ? 1 - : 0); - if (Op != OO_Call && Op != OO_Subscript && - ((NumParams == 1 && !CanBeUnaryOperator) || - (NumParams == 2 && !CanBeBinaryOperator) || (NumParams < 1) || - (NumParams > 2))) { - // We have the wrong number of parameters. - unsigned ErrorKind; - if (CanBeUnaryOperator && CanBeBinaryOperator) { - ErrorKind = 2; // 2 -> unary or binary. - } else if (CanBeUnaryOperator) { - ErrorKind = 0; // 0 -> unary - } else { - assert(CanBeBinaryOperator && - "All non-call overloaded operators are unary or binary!"); - ErrorKind = 1; // 1 -> binary - } - return Diag(FnDecl->getLocation(), diag::err_operator_overload_must_be) - << FnDecl->getDeclName() << NumParams << ErrorKind; - } - - if (Op == OO_Subscript && NumParams != 2) { - Diag(FnDecl->getLocation(), LangOpts.CPlusPlus23 - ? diag::ext_subscript_overload - : diag::error_subscript_overload) - << FnDecl->getDeclName() << (NumParams == 1 ? 0 : 2); - } - // Overloaded operators other than operator() and operator[] cannot be // variadic. if (Op != OO_Call && @@ -16543,32 +16566,20 @@ bool Sema::CheckOverloadedOperatorDeclaration(FunctionDecl *FnDecl) { } // Some operators must be member functions. - if (MustBeMemberOperator && !isa<CXXMethodDecl>(FnDecl)) { + static constexpr llvm::Bitset<NUM_OVERLOADED_OPERATORS> MemberOnlyOps{ +#define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary, MemberOnly) \ + MemberOnly ? OO_##Name : OO_None, +#include "clang/Basic/OperatorKinds.def" + }; + + if (MemberOnlyOps[Op] && !isa<CXXMethodDecl>(FnDecl)) return Diag(FnDecl->getLocation(), diag::err_operator_overload_must_be_member) - << FnDecl->getDeclName(); - } - - // C++ [over.inc]p1: - // The user-defined function called operator++ implements the - // prefix and postfix ++ operator. If this function is a member - // function with no parameters, or a non-member function with one - // parameter of class or enumeration type, it defines the prefix - // increment operator ++ for objects of that type. If the function - // is a member function with one parameter (which shall be of type - // int) or a non-member function with two parameters (the second - // of which shall be of type int), it defines the postfix - // increment operator ++ for objects of that type. - if ((Op == OO_PlusPlus || Op == OO_MinusMinus) && NumParams == 2) { - ParmVarDecl *LastParam = FnDecl->getParamDecl(FnDecl->getNumParams() - 1); - QualType ParamType = LastParam->getType(); + << FnDecl->getDeclName(); - if (!ParamType->isSpecificBuiltinType(BuiltinType::Int) && - !ParamType->isDependentType()) - return Diag(LastParam->getLocation(), - diag::err_operator_overload_post_incdec_must_be_int) - << LastParam->getType() << (Op == OO_MinusMinus); - } + if (!FnDecl->isFunctionTemplateSpecialization() && + CheckOverloadedOperatorParams(FnDecl)) + return true; return false; } diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp index e6ec4a7178e81..6eaa2a6a0b2e4 100644 --- a/clang/lib/Sema/SemaTemplateDeduction.cpp +++ b/clang/lib/Sema/SemaTemplateDeduction.cpp @@ -34,6 +34,7 @@ #include "clang/Basic/ExceptionSpecificationType.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/OperatorKinds.h" #include "clang/Basic/PartialDiagnostic.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/Specifiers.h" @@ -4120,6 +4121,28 @@ TemplateDeductionResult Sema::FinishTemplateArgumentDeduction( } } + // If the template is an operator function template, check that the + // resulting specialization is a valid operator function. + switch (Specialization->getOverloadedOperator()) { + case OO_None: + case OO_New: + case OO_Array_New: + case OO_Delete: + case OO_Array_Delete: + break; + + default: + // SFINAE does not apply at this point in the instantiation process. + // Push a new CodeSynthesisContext to briefly re-enable it here. + InstantiatingTemplate Inst( + *this, Info.getLocation(), FunctionTemplate, DeducedArgs, + CodeSynthesisContext::DeducedTemplateArgumentSubstitution, Info); + if (Inst.isInvalid()) + return TemplateDeductionResult::InstantiationDepth; + if (CheckOverloadedOperatorParams(Specialization)) + return TemplateDeductionResult::SubstitutionFailure; + } + // We skipped the instantiation of the explicit-specifier during the // substitution of `FD` before. So, we try to instantiate it back if // `Specialization` is either a constructor or a conversion function. diff --git a/clang/test/Parser/cxx20-coroutines.cpp b/clang/test/Parser/cxx20-coroutines.cpp index 7207fb98587ab..d0ea7cec4336f 100644 --- a/clang/test/Parser/cxx20-coroutines.cpp +++ b/clang/test/Parser/cxx20-coroutines.cpp @@ -30,6 +30,6 @@ void f(X x, Z z) { operator co_await(z); } -void operator co_await(); // expected-error {{must have at least one parameter}} +void operator co_await(); // expected-error {{must be a unary operator}} void operator co_await(X, Y, Z); // expected-error {{must be a unary operator}} void operator co_await(int); // expected-error {{parameter of class or enumeration type}} diff --git a/clang/test/SemaCXX/cxx2a-three-way-comparison.cpp b/clang/test/SemaCXX/cxx2a-three-way-comparison.cpp index b94225274fffb..5fc805ae395c5 100644 --- a/clang/test/SemaCXX/cxx2a-three-way-comparison.cpp +++ b/clang/test/SemaCXX/cxx2a-three-way-comparison.cpp @@ -13,7 +13,7 @@ struct A {}; constexpr int operator<=>(A a, A b) { return 42; } static_assert(operator<=>(A(), A()) == 42); -int operator<=>(); // expected-error {{overloaded 'operator<=>' must have at least one parameter of class or enumeration type}} +int operator<=>(); // expected-error {{overloaded 'operator<=>' must be a binary operator}} int operator<=>(A); // expected-error {{overloaded 'operator<=>' must be a binary operator}} int operator<=>(int, int); // expected-error {{overloaded 'operator<=>' must have at least one parameter of class or enumeration type}} int operator<=>(A, A, A); // expected-error {{overloaded 'operator<=>' must be a binary operator}} diff --git a/clang/test/SemaCXX/overloaded-operator-decl.cpp b/clang/test/SemaCXX/overloaded-operator-decl.cpp index 8a76c9251e419..ce54f166d4d67 100644 --- a/clang/test/SemaCXX/overloaded-operator-decl.cpp +++ b/clang/test/SemaCXX/overloaded-operator-decl.cpp @@ -21,6 +21,8 @@ void f(X x) { x = operator+(x, x); } +X operator+(); // expected-error {{overloaded 'operator+' must be a unary or binary operator}} + X operator+(int, float); // expected-error{{overloaded 'operator+' must have at least one parameter of class or enumeration type}} X operator*(X, X = 5); // expected-error{{parameter of overloaded 'operator*' cannot have a default argument}} @@ -69,33 +71,33 @@ class E { void operator+(E, ...) {} // expected-error{{overloaded 'operator+' cannot be variadic}} void operator-(E, ...) {} // expected-error{{overloaded 'operator-' cannot be variadic}} void operator*(E, ...) {} // expected-error{{overloaded 'operator*' cannot be variadic}} -void operator/(E, ...) {} // expected-error{{overloaded 'operator/' must be a binary operator}} +void operator/(E, ...) {} // expected-error{{overloaded 'operator/' cannot be variadic}} void operator/(E, E, ...) {} // expected-error{{overloaded 'operator/' cannot be variadic}} -void operator%(E, ...) {} // expected-error{{overloaded 'operator%' must be a binary operator}} +void operator%(E, ...) {} // expected-error{{overloaded 'operator%' cannot be variadic}} void operator%(E, E, ...) {} // expected-error{{overloaded 'operator%' cannot be variadic}} E& operator++(E&, ...); // expected-error{{overloaded 'operator++' cannot be variadic}} E& operator--(E&, ...); // expected-error{{overloaded 'operator--' cannot be variadic}} -bool operator<(const E& lhs, ...); // expected-error{{overloaded 'operator<' must be a binary operator}} +bool operator<(const E& lhs, ...); // expected-error{{overloaded 'operator<' cannot be variadic}} bool operator<(const E& lhs, const E& rhs, ...); // expected-error{{cannot be variadic}} -bool operator>(const E& lhs, ...); // expected-error{{overloaded 'operator>' must be a binary operator}} +bool operator>(const E& lhs, ...); // expected-error{{overloaded 'operator>' cannot be variadic}} bool operator>(const E& lhs, const E& rhs, ...); // expected-error{{cannot be variadic}} -bool operator>=(const E& lhs, ...); // expected-error{{overloaded 'operator>=' must be a binary operator}} +bool operator>=(const E& lhs, ...); // expected-error{{overloaded 'operator>=' cannot be variadic}} bool operator>=(const E& lhs, const E& rhs, ...); // expected-error{{cannot be variadic}} -bool operator<=(const E& lhs, ...); // expected-error{{overloaded 'operator<=' must be a binary operator}} +bool operator<=(const E& lhs, ...); // expected-error{{overloaded 'operator<=' cannot be variadic}} bool operator<=(const E& lhs, const E& rhs, ...); // expected-error{{cannot be variadic}} -bool operator==(const E& lhs, ...); // expected-error{{overloaded 'operator==' must be a binary operator}} +bool operator==(const E& lhs, ...); // expected-error{{overloaded 'operator==' cannot be variadic}} bool operator==(const E& lhs, const E& rhs, ...); // expected-error{{cannot be variadic}} -bool operator!=(const E& lhs, ...); // expected-error{{overloaded 'operator!=' must be a binary operator}} +bool operator!=(const E& lhs, ...); // expected-error{{overloaded 'operator!=' cannot be variadic}} bool operator!=(const E& lhs, const ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/131777 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
