[clang] [clang] Improve checking of operator functions (PR #131777)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 18 03:18:57 PDT 2025
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
More information about the cfe-commits
mailing list