Skip to content

[Clang] Static and explicit object member functions with the same parameter-type-lists #93430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,9 @@ C++23 Feature Support
- Added a ``__reference_converts_from_temporary`` builtin, completing the necessary compiler support for
`P2255R2: Type trait to determine if a reference binds to a temporary <https://wg21.link/P2255R2>`_.

- Implemented `P2797R0: Static and explicit object member functions with the same parameter-type-lists <https://wg21.link/P2797R0>`_.
This completes the support for "deducing this".

C++2c Feature Support
^^^^^^^^^^^^^^^^^^^^^

Expand Down
11 changes: 6 additions & 5 deletions clang/include/clang/AST/ExprCXX.h
Original file line number Diff line number Diff line change
Expand Up @@ -3025,9 +3025,10 @@ class OverloadExpr : public Expr {

public:
struct FindResult {
OverloadExpr *Expression;
bool IsAddressOfOperand;
bool HasFormOfMemberPointer;
OverloadExpr *Expression = nullptr;
bool IsAddressOfOperand = false;
bool IsAddressOfOperandWithParen = false;
bool HasFormOfMemberPointer = false;
};

/// Finds the overloaded expression in the given expression \p E of
Expand All @@ -3039,6 +3040,7 @@ class OverloadExpr : public Expr {
assert(E->getType()->isSpecificBuiltinType(BuiltinType::Overload));

FindResult Result;
bool HasParen = isa<ParenExpr>(E);

E = E->IgnoreParens();
if (isa<UnaryOperator>(E)) {
Expand All @@ -3048,10 +3050,9 @@ class OverloadExpr : public Expr {

Result.HasFormOfMemberPointer = (E == Ovl && Ovl->getQualifier());
Result.IsAddressOfOperand = true;
Result.IsAddressOfOperandWithParen = HasParen;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup nit: I would vastly prefer FindResult have a constructor & deleted default constructor. This seems like it would be really easy to forget to initialize on some branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a default member initializer for everything

Result.Expression = Ovl;
} else {
Result.HasFormOfMemberPointer = false;
Result.IsAddressOfOperand = false;
Result.Expression = cast<OverloadExpr>(E);
}

Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Sema/Overload.h
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,8 @@ class Sema;
/// object argument.
bool IgnoreObjectArgument : 1;

bool TookAddressOfOverload : 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @Endilll -- it might be nice to refactor this code to use LLVM_PREFERRED_TYPE, but also to fix the bit-field types so these all pack together consistently.


/// True if the candidate was found using ADL.
CallExpr::ADLCallKind IsADLCandidate : 1;

Expand Down Expand Up @@ -999,6 +1001,10 @@ class Sema;
/// Initialization of an object of class type by constructor,
/// using either a parenthesized or braced list of arguments.
CSK_InitByConstructor,

/// C++ [over.match.call.general]
/// Resolve a call through the address of an overload set.
CSK_AddressOfOverloadSet,
};

/// Information about operator rewrites to consider when adding operator
Expand Down
27 changes: 25 additions & 2 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5813,6 +5813,27 @@ static TypoCorrection TryTypoCorrectionForCall(Sema &S, Expr *Fn,
return TypoCorrection();
}

// [C++26][[expr.unary.op]/p4
// A pointer to member is only formed when an explicit &
// is used and its operand is a qualified-id not enclosed in parentheses.
static bool isParenthetizedAndQualifiedAddressOfExpr(Expr *Fn) {
if (!isa<ParenExpr>(Fn))
return false;

Fn = Fn->IgnoreParens();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also ignore implicit casts along the way here for all of these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, Any cast would make it a different shape than (&foo::bar)


auto *UO = dyn_cast<UnaryOperator>(Fn);
if (!UO || UO->getOpcode() != clang::UO_AddrOf)
return false;
if (auto *DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr()->IgnoreParens())) {
assert(isa<FunctionDecl>(DRE->getDecl()) && "expected a function");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cor3ntin, this assert is failing for a simple test (https://godbolt.org/z/Ye88sxfo8):

int bar(void) { return 55; }
int (&fref)(void) = bar;

int main(void) {
  return (&fref)();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. I'll get to it this week. Thanks for letting me know

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping re: regression (assertion failure) @cor3ntin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... I completely forgot (busy month!), thanks for reminding me.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another friendly ping @cor3ntin.
I see dd82a84 landed.

return DRE->hasQualifier();
}
if (auto *OVL = dyn_cast<OverloadExpr>(UO->getSubExpr()->IgnoreParens()))
return OVL->getQualifier();
return false;
}

/// ConvertArgumentsForCall - Converts the arguments specified in
/// Args/NumArgs to the parameter types of the function FDecl with
/// function prototype Proto. Call is the call expression itself, and
Expand All @@ -5834,8 +5855,10 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,

// C99 6.5.2.2p7 - the arguments are implicitly converted, as if by
// assignment, to the types of the corresponding parameter, ...

bool AddressOf = isParenthetizedAndQualifiedAddressOfExpr(Fn);
bool HasExplicitObjectParameter =
FDecl && FDecl->hasCXXExplicitFunctionObjectParameter();
!AddressOf && FDecl && FDecl->hasCXXExplicitFunctionObjectParameter();
unsigned ExplicitObjectParameterOffset = HasExplicitObjectParameter ? 1 : 0;
unsigned NumParams = Proto->getNumParams();
bool Invalid = false;
Expand Down Expand Up @@ -6546,7 +6569,7 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc,
OverloadExpr::FindResult find = OverloadExpr::find(Fn);

// We aren't supposed to apply this logic if there's an '&' involved.
if (!find.HasFormOfMemberPointer) {
if (!find.HasFormOfMemberPointer || find.IsAddressOfOperandWithParen) {
if (Expr::hasAnyTypeDependentArguments(ArgExprs))
return CallExpr::Create(Context, Fn, ArgExprs, Context.DependentTy,
VK_PRValue, RParenLoc, CurFPFeatureOverrides());
Expand Down
110 changes: 91 additions & 19 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6971,6 +6971,7 @@ void Sema::AddOverloadCandidate(
Candidate.IsSurrogate = false;
Candidate.IsADLCandidate = IsADLCandidate;
Candidate.IgnoreObjectArgument = false;
Candidate.TookAddressOfOverload = false;
Candidate.ExplicitCallArguments = Args.size();

// Explicit functions are not actually candidates at all if we're not
Expand Down Expand Up @@ -7545,10 +7546,24 @@ Sema::AddMethodCandidate(CXXMethodDecl *Method, DeclAccessPair FoundDecl,
CandidateSet.getRewriteInfo().getRewriteKind(Method, PO);
Candidate.IsSurrogate = false;
Candidate.IgnoreObjectArgument = false;
Candidate.TookAddressOfOverload =
CandidateSet.getKind() == OverloadCandidateSet::CSK_AddressOfOverloadSet;
Candidate.ExplicitCallArguments = Args.size();

unsigned NumParams = Method->getNumExplicitParams();
unsigned ExplicitOffset = Method->isExplicitObjectMemberFunction() ? 1 : 0;
bool IgnoreExplicitObject =
(Method->isExplicitObjectMemberFunction() &&
CandidateSet.getKind() ==
OverloadCandidateSet::CSK_AddressOfOverloadSet);
bool ImplicitObjectMethodTreatedAsStatic =
CandidateSet.getKind() ==
OverloadCandidateSet::CSK_AddressOfOverloadSet &&
Method->isImplicitObjectMemberFunction();

unsigned ExplicitOffset =
!IgnoreExplicitObject && Method->isExplicitObjectMemberFunction() ? 1 : 0;

unsigned NumParams = Method->getNumParams() - ExplicitOffset +
int(ImplicitObjectMethodTreatedAsStatic);

// (C++ 13.3.2p2): A candidate function having fewer than m
// parameters is viable only if it has an ellipsis in its parameter
Expand All @@ -7566,7 +7581,10 @@ Sema::AddMethodCandidate(CXXMethodDecl *Method, DeclAccessPair FoundDecl,
// (8.3.6). For the purposes of overload resolution, the
// parameter list is truncated on the right, so that there are
// exactly m parameters.
unsigned MinRequiredArgs = Method->getMinRequiredExplicitArguments();
unsigned MinRequiredArgs = Method->getMinRequiredArguments() -
ExplicitOffset +
int(ImplicitObjectMethodTreatedAsStatic);

if (Args.size() < MinRequiredArgs && !PartialOverloading) {
// Not enough arguments.
Candidate.Viable = false;
Expand Down Expand Up @@ -7636,7 +7654,14 @@ Sema::AddMethodCandidate(CXXMethodDecl *Method, DeclAccessPair FoundDecl,
// exist for each argument an implicit conversion sequence
// (13.3.3.1) that converts that argument to the corresponding
// parameter of F.
QualType ParamType = Proto->getParamType(ArgIdx + ExplicitOffset);
QualType ParamType;
if (ImplicitObjectMethodTreatedAsStatic) {
ParamType = ArgIdx == 0
? Method->getFunctionObjectParameterReferenceType()
: Proto->getParamType(ArgIdx - 1);
} else {
ParamType = Proto->getParamType(ArgIdx + ExplicitOffset);
}
Candidate.Conversions[ConvIdx]
= TryCopyInitialization(*this, Args[ArgIdx], ParamType,
SuppressUserConversions,
Expand Down Expand Up @@ -7717,6 +7742,7 @@ void Sema::AddMethodTemplateCandidate(
Candidate.IgnoreObjectArgument =
cast<CXXMethodDecl>(Candidate.Function)->isStatic() ||
ObjectType.isNull();
Candidate.TookAddressOfOverload = false;
Candidate.ExplicitCallArguments = Args.size();
if (Result == TemplateDeductionResult::NonDependentConversionFailure)
Candidate.FailureKind = ovl_fail_bad_conversion;
Expand Down Expand Up @@ -7807,6 +7833,7 @@ void Sema::AddTemplateOverloadCandidate(
Candidate.IgnoreObjectArgument =
isa<CXXMethodDecl>(Candidate.Function) &&
!isa<CXXConstructorDecl>(Candidate.Function);
Candidate.TookAddressOfOverload = false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel like Candidate should have a deleted default ctor/constructor pair too :D For exactly this reason.

Candidate.ExplicitCallArguments = Args.size();
if (Result == TemplateDeductionResult::NonDependentConversionFailure)
Candidate.FailureKind = ovl_fail_bad_conversion;
Expand Down Expand Up @@ -7998,6 +8025,7 @@ void Sema::AddConversionCandidate(
Candidate.Function = Conversion;
Candidate.IsSurrogate = false;
Candidate.IgnoreObjectArgument = false;
Candidate.TookAddressOfOverload = false;
Candidate.FinalConversion.setAsIdentityConversion();
Candidate.FinalConversion.setFromType(ConvType);
Candidate.FinalConversion.setAllToTypes(ToType);
Expand Down Expand Up @@ -8200,6 +8228,7 @@ void Sema::AddTemplateConversionCandidate(
Candidate.FailureKind = ovl_fail_bad_deduction;
Candidate.IsSurrogate = false;
Candidate.IgnoreObjectArgument = false;
Candidate.TookAddressOfOverload = false;
Candidate.ExplicitCallArguments = 1;
Candidate.DeductionFailure = MakeDeductionFailureInfo(Context, Result,
Info);
Expand Down Expand Up @@ -8240,6 +8269,7 @@ void Sema::AddSurrogateCandidate(CXXConversionDecl *Conversion,
Candidate.Viable = true;
Candidate.IsSurrogate = true;
Candidate.IgnoreObjectArgument = false;
Candidate.TookAddressOfOverload = false;
Candidate.ExplicitCallArguments = Args.size();

// Determine the implicit conversion sequence for the implicit
Expand Down Expand Up @@ -8465,6 +8495,7 @@ void Sema::AddBuiltinCandidate(QualType *ParamTys, ArrayRef<Expr *> Args,
Candidate.Function = nullptr;
Candidate.IsSurrogate = false;
Candidate.IgnoreObjectArgument = false;
Candidate.TookAddressOfOverload = false;
std::copy(ParamTys, ParamTys + Args.size(), Candidate.BuiltinParamTypes);

// Determine the implicit conversion sequences for each of the
Expand Down Expand Up @@ -10929,6 +10960,12 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc,
if (Best->Function && Best->Function->isDeleted())
return OR_Deleted;

if (auto *M = dyn_cast_or_null<CXXMethodDecl>(Best->Function);
Kind == CSK_AddressOfOverloadSet && M &&
M->isImplicitObjectMemberFunction()) {
return OR_No_Viable_Function;
}

if (!EquivalentCands.empty())
S.diagnoseEquivalentInternalLinkageDeclarations(Loc, Best->Function,
EquivalentCands);
Expand Down Expand Up @@ -11508,9 +11545,10 @@ static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
/// candidates. This is not covered by the more general DiagnoseArityMismatch()
/// over a candidate in any candidate set.
static bool CheckArityMismatch(Sema &S, OverloadCandidate *Cand,
unsigned NumArgs) {
unsigned NumArgs, bool IsAddressOf = false) {
FunctionDecl *Fn = Cand->Function;
unsigned MinParams = Fn->getMinRequiredArguments();
unsigned MinParams = Fn->getMinRequiredExplicitArguments() +
((IsAddressOf && !Fn->isStatic()) ? 1 : 0);

// With invalid overloaded operators, it's possible that we think we
// have an arity mismatch when in fact it looks like we have the
Expand Down Expand Up @@ -11538,7 +11576,8 @@ static bool CheckArityMismatch(Sema &S, OverloadCandidate *Cand,

/// General arity mismatch diagnosis over a candidate in a candidate set.
static void DiagnoseArityMismatch(Sema &S, NamedDecl *Found, Decl *D,
unsigned NumFormalArgs) {
unsigned NumFormalArgs,
bool IsAddressOf = false) {
assert(isa<FunctionDecl>(D) &&
"The templated declaration should at least be a function"
" when diagnosing bad template argument deduction due to too many"
Expand All @@ -11548,12 +11587,17 @@ static void DiagnoseArityMismatch(Sema &S, NamedDecl *Found, Decl *D,

// TODO: treat calls to a missing default constructor as a special case
const auto *FnTy = Fn->getType()->castAs<FunctionProtoType>();
unsigned MinParams = Fn->getMinRequiredExplicitArguments();
unsigned MinParams = Fn->getMinRequiredExplicitArguments() +
((IsAddressOf && !Fn->isStatic()) ? 1 : 0);

// at least / at most / exactly
bool HasExplicitObjectParam = Fn->hasCXXExplicitFunctionObjectParameter();
unsigned ParamCount = FnTy->getNumParams() - (HasExplicitObjectParam ? 1 : 0);
bool HasExplicitObjectParam =
!IsAddressOf && Fn->hasCXXExplicitFunctionObjectParameter();

unsigned ParamCount =
Fn->getNumNonObjectParams() + ((IsAddressOf && !Fn->isStatic()) ? 1 : 0);
unsigned mode, modeCount;

if (NumFormalArgs < MinParams) {
if (MinParams != ParamCount || FnTy->isVariadic() ||
FnTy->isTemplateVariadic())
Expand All @@ -11573,7 +11617,7 @@ static void DiagnoseArityMismatch(Sema &S, NamedDecl *Found, Decl *D,
std::pair<OverloadCandidateKind, OverloadCandidateSelect> FnKindPair =
ClassifyOverloadCandidate(S, Found, Fn, CRK_None, Description);

if (modeCount == 1 &&
if (modeCount == 1 && !IsAddressOf &&
Fn->getParamDecl(HasExplicitObjectParam ? 1 : 0)->getDeclName())
S.Diag(Fn->getLocation(), diag::note_ovl_candidate_arity_one)
<< (unsigned)FnKindPair.first << (unsigned)FnKindPair.second
Expand All @@ -11592,8 +11636,9 @@ static void DiagnoseArityMismatch(Sema &S, NamedDecl *Found, Decl *D,
/// Arity mismatch diagnosis specific to a function overload candidate.
static void DiagnoseArityMismatch(Sema &S, OverloadCandidate *Cand,
unsigned NumFormalArgs) {
if (!CheckArityMismatch(S, Cand, NumFormalArgs))
DiagnoseArityMismatch(S, Cand->FoundDecl, Cand->Function, NumFormalArgs);
if (!CheckArityMismatch(S, Cand, NumFormalArgs, Cand->TookAddressOfOverload))
DiagnoseArityMismatch(S, Cand->FoundDecl, Cand->Function, NumFormalArgs,
Cand->TookAddressOfOverload);
}

static TemplateDecl *getDescribedTemplate(Decl *Templated) {
Expand Down Expand Up @@ -12033,6 +12078,13 @@ static void NoteFunctionCandidate(Sema &S, OverloadCandidate *Cand,
Cand->FailureKind != ovl_fail_bad_conversion)
return;

// Skip implicit member functions when trying to resolve
// the address of a an overload set for a function pointer.
if (Cand->TookAddressOfOverload &&
!Cand->Function->hasCXXExplicitFunctionObjectParameter() &&
!Cand->Function->isStatic())
return;

// Note deleted candidates, but only if they're viable.
if (Cand->Viable) {
if (Fn->isDeleted()) {
Expand Down Expand Up @@ -14076,6 +14128,21 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
}

case OR_No_Viable_Function: {
if (*Best != CandidateSet->end() &&
CandidateSet->getKind() ==
clang::OverloadCandidateSet::CSK_AddressOfOverloadSet) {
if (CXXMethodDecl *M =
dyn_cast_if_present<CXXMethodDecl>((*Best)->Function);
M && M->isImplicitObjectMemberFunction()) {
CandidateSet->NoteCandidates(
PartialDiagnosticAt(
Fn->getBeginLoc(),
SemaRef.PDiag(diag::err_member_call_without_object) << 0 << M),
SemaRef, OCD_AmbiguousCandidates, Args);
return ExprError();
}
}

// Try to recover by looking for viable functions which the user might
// have meant to call.
ExprResult Recovery = BuildRecoveryCallExpr(SemaRef, S, Fn, ULE, LParenLoc,
Expand Down Expand Up @@ -14167,8 +14234,10 @@ ExprResult Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,
Expr *ExecConfig,
bool AllowTypoCorrection,
bool CalleesAddressIsTaken) {
OverloadCandidateSet CandidateSet(Fn->getExprLoc(),
OverloadCandidateSet::CSK_Normal);
OverloadCandidateSet CandidateSet(
Fn->getExprLoc(), CalleesAddressIsTaken
? OverloadCandidateSet::CSK_AddressOfOverloadSet
: OverloadCandidateSet::CSK_Normal);
ExprResult result;

if (buildOverloadedCallSet(S, Fn, ULE, Args, LParenLoc, &CandidateSet,
Expand Down Expand Up @@ -16333,9 +16402,9 @@ ExprResult Sema::FixOverloadedFunctionReference(Expr *E, DeclAccessPair Found,
assert(UnOp->getOpcode() == UO_AddrOf &&
"Can only take the address of an overloaded function");
if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(Fn)) {
if (Method->isStatic()) {
// Do nothing: static member functions aren't any different
// from non-member functions.
if (!Method->isImplicitObjectMemberFunction()) {
// Do nothing: the address of static and
// explicit object member functions is a (non-member) function pointer.
} else {
// Fix the subexpression, which really has to be an
// UnresolvedLookupExpr holding an overloaded member function
Expand Down Expand Up @@ -16393,7 +16462,10 @@ ExprResult Sema::FixOverloadedFunctionReference(Expr *E, DeclAccessPair Found,
}

QualType Type = Fn->getType();
ExprValueKind ValueKind = getLangOpts().CPlusPlus ? VK_LValue : VK_PRValue;
ExprValueKind ValueKind =
getLangOpts().CPlusPlus && !Fn->hasCXXExplicitFunctionObjectParameter()
? VK_LValue
: VK_PRValue;

// FIXME: Duplicated from BuildDeclarationNameExpr.
if (unsigned BID = Fn->getBuiltinID()) {
Expand Down
Loading
Loading