[Clang] Static and explicit object member functions with the same parameter-type-lists #93430
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ameter-type-lists Implement wg21.link/P2797. Because taking the address of an explicit object member function results in a function pointer, a call expression where the id-expression is an explicit object member is made to behave consistently with that model. This change forces clang to perform overload resolution in the presence of an ixpression of the form `(&Foo::bar)(args...)`, which we previously failed to do consistently.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: cor3ntin (cor3ntin) ChangesImplement wg21.link/P2797. Because taking the address of an explicit object member function results in a function pointer, a call expression where the id-expression is an explicit object member is made to behave consistently with that model. This change forces clang to perform overload resolution in the presence of an id-expression of the form Patch is 20.88 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/93430.diff 12 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 825e91876ffce..0a945a9989b0d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -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
^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index dbf693611a7fa..557e9fd99c293 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -3027,6 +3027,7 @@ class OverloadExpr : public Expr {
struct FindResult {
OverloadExpr *Expression;
bool IsAddressOfOperand;
+ bool IsAddressOfOperandWithParen;
bool HasFormOfMemberPointer;
};
@@ -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)) {
@@ -3048,10 +3050,12 @@ class OverloadExpr : public Expr {
Result.HasFormOfMemberPointer = (E == Ovl && Ovl->getQualifier());
Result.IsAddressOfOperand = true;
+ Result.IsAddressOfOperandWithParen = HasParen;
Result.Expression = Ovl;
} else {
Result.HasFormOfMemberPointer = false;
Result.IsAddressOfOperand = false;
+ Result.IsAddressOfOperandWithParen = false;
Result.Expression = cast<OverloadExpr>(E);
}
diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index 76311b00d2fc5..64cdd6cdf043d 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -899,6 +899,8 @@ class Sema;
/// object argument.
bool IgnoreObjectArgument : 1;
+ bool TookAddressOfOverload : 1;
+
/// True if the candidate was found using ADL.
CallExpr::ADLCallKind IsADLCandidate : 1;
@@ -999,6 +1001,8 @@ class Sema;
/// Initialization of an object of class type by constructor,
/// using either a parenthesized or braced list of arguments.
CSK_InitByConstructor,
+
+ CSK_AddressOfOverloadSet,
};
/// Information about operator rewrites to consider when adding operator
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 410f80ae864a1..15496f3323b02 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5813,6 +5813,24 @@ static TypoCorrection TryTypoCorrectionForCall(Sema &S, Expr *Fn,
return TypoCorrection();
}
+static bool isParenthetizedAndQualifiedAddressOfExpr(Expr *Fn) {
+ if (!isa<ParenExpr>(Fn))
+ return false;
+
+ Fn = Fn->IgnoreParens();
+ auto *UO = dyn_cast<UnaryOperator>(Fn);
+ if (!UO)
+ return false;
+ assert(cast<UnaryOperator>(Fn)->getOpcode() == UO_AddrOf);
+ if (auto *DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr()->IgnoreParens())) {
+ 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
@@ -5834,9 +5852,12 @@ 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();
- unsigned ExplicitObjectParameterOffset = HasExplicitObjectParameter ? 1 : 0;
+ !AddressOf && FDecl && FDecl->hasCXXExplicitFunctionObjectParameter();
+ unsigned ExplicitObjectParameterOffset =
+ HasExplicitObjectParameter && !AddressOf ? 1 : 0;
unsigned NumParams = Proto->getNumParams();
bool Invalid = false;
unsigned MinArgs = FDecl ? FDecl->getMinRequiredArguments() : NumParams;
@@ -6546,7 +6567,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());
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 0c89fca8d38eb..17cccefa8e929 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -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
@@ -7545,10 +7546,19 @@ 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);
+ unsigned ExplicitOffset =
+ !IgnoreExplicitObject && Method->isExplicitObjectMemberFunction() ? 1 : 0;
+ unsigned NumParams = Method->getNumParams() - ExplicitOffset;
+ if (CandidateSet.getKind() == OverloadCandidateSet::CSK_AddressOfOverloadSet)
+ NumParams += int(Method->isImplicitObjectMemberFunction());
// (C++ 13.3.2p2): A candidate function having fewer than m
// parameters is viable only if it has an ellipsis in its parameter
@@ -7566,7 +7576,12 @@ 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;
+ if (CandidateSet.getKind() ==
+ OverloadCandidateSet::CSK_AddressOfOverloadSet &&
+ Method->isImplicitObjectMemberFunction())
+ MinRequiredArgs++;
+
if (Args.size() < MinRequiredArgs && !PartialOverloading) {
// Not enough arguments.
Candidate.Viable = false;
@@ -7636,7 +7651,16 @@ 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 (CandidateSet.getKind() ==
+ OverloadCandidateSet::CSK_AddressOfOverloadSet &&
+ Method->isImplicitObjectMemberFunction()) {
+ ParamType = ArgIdx == 0
+ ? Method->getFunctionObjectParameterReferenceType()
+ : Proto->getParamType(ArgIdx - 1);
+ } else {
+ ParamType = Proto->getParamType(ArgIdx + ExplicitOffset);
+ }
Candidate.Conversions[ConvIdx]
= TryCopyInitialization(*this, Args[ArgIdx], ParamType,
SuppressUserConversions,
@@ -7998,6 +8022,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);
@@ -8200,6 +8225,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);
@@ -8240,6 +8266,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
@@ -8465,6 +8492,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
@@ -10929,6 +10957,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);
@@ -11538,7 +11572,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"
@@ -11551,7 +11586,8 @@ static void DiagnoseArityMismatch(Sema &S, NamedDecl *Found, Decl *D,
unsigned MinParams = Fn->getMinRequiredExplicitArguments();
// at least / at most / exactly
- bool HasExplicitObjectParam = Fn->hasCXXExplicitFunctionObjectParameter();
+ bool HasExplicitObjectParam =
+ !IsAddressOf && Fn->hasCXXExplicitFunctionObjectParameter();
unsigned ParamCount = FnTy->getNumParams() - (HasExplicitObjectParam ? 1 : 0);
unsigned mode, modeCount;
if (NumFormalArgs < MinParams) {
@@ -11593,7 +11629,8 @@ static void DiagnoseArityMismatch(Sema &S, NamedDecl *Found, Decl *D,
static void DiagnoseArityMismatch(Sema &S, OverloadCandidate *Cand,
unsigned NumFormalArgs) {
if (!CheckArityMismatch(S, Cand, NumFormalArgs))
- DiagnoseArityMismatch(S, Cand->FoundDecl, Cand->Function, NumFormalArgs);
+ DiagnoseArityMismatch(S, Cand->FoundDecl, Cand->Function, NumFormalArgs,
+ Cand->TookAddressOfOverload);
}
static TemplateDecl *getDescribedTemplate(Decl *Templated) {
@@ -14076,6 +14113,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,
@@ -14167,8 +14219,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,
@@ -16333,7 +16387,7 @@ 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()) {
+ if (!Method->isImplicitObjectMemberFunction()) {
// Do nothing: static member functions aren't any different
// from non-member functions.
} else {
diff --git a/clang/test/CXX/drs/cwg1xx.cpp b/clang/test/CXX/drs/cwg1xx.cpp
index a8f9b705a9866..21859fc6b1cbf 100644
--- a/clang/test/CXX/drs/cwg1xx.cpp
+++ b/clang/test/CXX/drs/cwg1xx.cpp
@@ -843,23 +843,21 @@ namespace cwg161 { // cwg161: 3.1
};
}
-namespace cwg162 { // cwg162: no
+namespace cwg162 { // cwg162: 19
struct A {
char &f(char);
static int &f(int);
void g() {
int &a = (&A::f)(0);
- // FIXME: expected-error@-1 {{reference to overloaded function could not be resolved; did you mean to call it?}}
char &b = (&A::f)('0');
- // expected-error@-1 {{reference to overloaded function could not be resolved; did you mean to call it?}}
+ // expected-error@-1 {{non-const lvalue reference to type 'char' cannot bind to a value of unrelated type 'int'}}
}
};
int &c = (&A::f)(0);
- // FIXME: expected-error@-1 {{reference to overloaded function could not be resolved; did you mean to call it?}}
char &d = (&A::f)('0');
- // expected-error@-1 {{reference to overloaded function could not be resolved; did you mean to call it?}}
+ // expected-error@-1 {{non-const lvalue reference to type 'char' cannot bind to a value of unrelated type 'int'}}
}
// cwg163: na
diff --git a/clang/test/CXX/drs/cwg26xx.cpp b/clang/test/CXX/drs/cwg26xx.cpp
index d3c5b5bb7b6b9..6c91377643fb9 100644
--- a/clang/test/CXX/drs/cwg26xx.cpp
+++ b/clang/test/CXX/drs/cwg26xx.cpp
@@ -240,3 +240,29 @@ void test() {
}
}
#endif
+
+
+#if __cplusplus >= 202302L
+namespace cwg2692 { // cwg2692: 19
+
+ struct A {
+ static void f(A); // #cwg2692-1
+ void f(this A); // #cwg2692-2
+
+ void g();
+ };
+
+ void A::g() {
+ (&A::f)(A()); // expected-error {{call to 'f' is ambiguous}}
+ // expected-note@#cwg2692-1 {{candidate}}
+ // expected-note@#cwg2692-2 {{candidate}}
+
+
+
+ (&A::f)(); // expected-error {{no matching function for call to 'f'}}
+ // expected-note@#cwg2692-1 {{candidate function not viable: requires 1 argument, but 0 were provided}}
+ // expected-note@#cwg2692-2 {{candidate function not viable: requires at most 1 argument, but 0 were provided}}
+ }
+
+}
+#endif
diff --git a/clang/test/CXX/drs/cwg2771.cpp b/clang/test/CXX/drs/cwg2771.cpp
new file mode 100644
index 0000000000000..e877e6fe4a5fe
--- /dev/null
+++ b/clang/test/CXX/drs/cwg2771.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++23 %s -ast-dump | FileCheck --check-prefixes=CXX23 %s
+
+namespace cwg2771 { // cwg2771: 18
+
+struct A{
+ int a;
+ void cwg2771(){
+ int* r = &a;
+ }
+};
+// CXX23: CXXMethodDecl{{.+}}cwg2771
+// CXX23-NEXT: CompoundStmt
+// CXX23-NEXT: DeclStmt
+// CXX23-NEXT: VarDecl
+// CXX23-NEXT: UnaryOperator
+// CXX23-NEXT: MemberExpr
+// CXX23-NEXT: CXXThisExpr{{.+}}'cwg2771::A *'
+}
diff --git a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp
index 649fe2afbf4e9..f9f9fbd7397f8 100644
--- a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp
+++ b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp
@@ -245,3 +245,23 @@ void f() {
d();
}
}
+
+
+namespace P2797 {
+struct C {
+ void c(this const C&); // #first
+ void c() &; // #second
+ static void c(int = 0); // #third
+
+ void d() {
+ (&C::c)(C{});
+ (&C::c)();
+ }
+};
+void test() {
+ (void)C{}.d();
+}
+// CHECK-LABEL: {{.*}} @_ZN5P27971C1dEv
+// CHECK: call void @_ZNH5P27971C1cERKS0_
+// CHECK: call void @_ZN5P27971C1cEi
+}
diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
index cdb9d1324b974..86046497c2ef2 100644
--- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp
+++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
@@ -893,3 +893,30 @@ void g() {
a * lval;
}
}
+
+namespace P2797 {
+struct C {
+ void c(this const C&); // #first
+ void c() &; // #second
+ static void c(int = 0); // #third
+
+ void d() {
+ c(); // expected-error {{call to member function 'c' is ambiguous}}
+ // expected-note@#first {{candidate function}}
+ // expected-note@#second {{candidate function}}
+ // expected-note@#third {{candidate function}}
+
+ (C::c)(); // expected-error {{call to member function 'c' is ambiguous}}
+ // expected-note@#first {{candidate function}}
+ // expected-note@#second {{candidate function}}
+ // expected-note@#third {{candidate function}}
+
+ (&(C::c))(); // expected-error {{cannot create a non-constant pointer to member function}}
+ (&C::c)(C{});
+ (&C::c)(*this); // expected-error {{call to non-static member function without an object argument}}
+ // expected-note@#second {{candidate function}}
+
+ (&C::c)();
+ }
+};
+}
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 4cce88fe0490f..7df6c0a05a487 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -1010,7 +1010,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/162.html">162</a></td>
<td>CD1</td>
<td>(<TT>&C::f)()</TT> with nonstatic members</td>
- <td class="none" align="center">No</td>
+ <td class="unreleased" align="center">Clang 19</td>
</tr>
<tr id="163">
<td><a href="https://cplusplus.github.io/CWG/issues/163.html">163</a></td>
@@ -15960,7 +15960,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2692.html">2692</a></td>
<td>C++23</td>
<td>Static and explicit object member functions with the same parameter-type-lists</td>
- <td class="unknown" align="center">Unknown</td>
+ <td class="unreleased" align="center">Clang 19</td>
</tr>
<tr class="open" id="2693">
<td><a href="https://cplusplus.github.io/CWG/issues/2693.html">2693</a></td>
@@ -16435,7 +16435,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a ...
[truncated]
|
Endilll
reviewed
May 27, 2024
| struct A{ | ||
| int a; | ||
| void cwg2771(){ | ||
| int* r = &a; |
There was a problem hiding this comment.
You can use #pragma clang __debug dump &a to match less AST in FileCheck.
Endilll
reviewed
May 27, 2024
Sirraide
reviewed
May 27, 2024
|
@AaronBallman @erichkeane ping! |
erichkeane
approved these changes
Jun 4, 2024
| @@ -3048,10 +3050,12 @@ class OverloadExpr : public Expr { | |||
|
|
|||
| Result.HasFormOfMemberPointer = (E == Ovl && Ovl->getQualifier()); | |||
| Result.IsAddressOfOperand = true; | |||
| Result.IsAddressOfOperandWithParen = HasParen; | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I added a default member initializer for everything
| if (!isa<ParenExpr>(Fn)) | ||
| return false; | ||
|
|
||
| Fn = Fn->IgnoreParens(); |
There was a problem hiding this comment.
Should we also ignore implicit casts along the way here for all of these?
There was a problem hiding this comment.
Nope, Any cast would make it a different shape than (&foo::bar)
| @@ -7807,6 +7833,7 @@ void Sema::AddTemplateOverloadCandidate( | |||
| Candidate.IgnoreObjectArgument = | |||
| isa<CXXMethodDecl>(Candidate.Function) && | |||
| !isa<CXXConstructorDecl>(Candidate.Function); | |||
| Candidate.TookAddressOfOverload = false; | |||
There was a problem hiding this comment.
Feel like Candidate should have a deleted default ctor/constructor pair too :D For exactly this reason.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
AaronBallman
reviewed
Jun 4, 2024
| @@ -899,6 +899,8 @@ class Sema; | |||
| /// object argument. | |||
| bool IgnoreObjectArgument : 1; | |||
|
|
|||
| bool TookAddressOfOverload : 1; | |||
There was a problem hiding this comment.
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.
|
Hit an msan use-of-uninitialized on an internal use case (a tool that uses Clang via API). the test was compiling this source code: and failed in this stack: I realize this isn't exactly repro steps - the included is from libc++, for what that's worth - I don't /think/ this build is using modules. But providing this info in case it's sufficient information to diagnose the issue. It seems to be the TookAddressOfOverload variable is used-uninitialized somewhere in this path. (I don't actually have a clang-built-with-msan to test this further myself at the moment, but trying to figure out some bootstrapping issues to try to help narrow it down) |
|
@cor3ntin ping on this? |
|
Please open an issue, I am in a WG21 meeting for the next week. Sorry about that. |
11 tasks
| 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"); |
There was a problem hiding this comment.
@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)();
}There was a problem hiding this comment.
Ack. I'll get to it this week. Thanks for letting me know
There was a problem hiding this comment.
Ping re: regression (assertion failure) @cor3ntin
There was a problem hiding this comment.
... I completely forgot (busy month!), thanks for reminding me.
cor3ntin
added a commit
to cor3ntin/llvm-project
that referenced
this pull request
Jul 24, 2024
The PR llvm#93430 introduced an assertion that did not make any sense. and caused a regression. The fix is to simply remove the assertion. No changelog. the intent is to backport this fix to clang 19.
cor3ntin
added a commit
to cor3ntin/llvm-project
that referenced
this pull request
Jul 24, 2024
Initialize some fields of OverloadCandidate in its constructor. The goal here is try to fix read of uninitialized variable (which I was not able to reproduce) llvm#93430 (comment) We should certainly try to improve the construction of `OverloadCandidate` further as it can be quite britle.
cor3ntin
added a commit
to cor3ntin/llvm-project
that referenced
this pull request
Jul 24, 2024
Initialize some fields of OverloadCandidate in its constructor. The goal here is try to fix read of uninitialized variable (which I was not able to reproduce) llvm#93430 (comment) We should certainly try to improve the construction of `OverloadCandidate` further as it can be quite britle.
cor3ntin
added a commit
to cor3ntin/llvm-project
that referenced
this pull request
Jul 24, 2024
Initialize some fields of OverloadCandidate in its constructor. The goal here is try to fix read of uninitialized variable (which I was not able to reproduce) llvm#93430 (comment) We should certainly try to improve the construction of `OverloadCandidate` further as it can be quite britle.
cor3ntin
added a commit
that referenced
this pull request
Jul 24, 2024
…100318) Initialize some fields of OverloadCandidate in its constructor. The goal here is try to fix read of uninitialized variable (which I was not able to reproduce) #93430 (comment) We should certainly try to improve the construction of `OverloadCandidate` further as it can be quite britle.
cor3ntin
added a commit
that referenced
this pull request
Jul 24, 2024
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Jul 24, 2024
The PR llvm#93430 introduced an assertion that did not make any sense. and caused a regression. The fix is to simply remove the assertion. No changelog. the intent is to backport this fix to clang 19. (cherry picked from commit dd82a84)
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Jul 24, 2024
…lvm#100318) Initialize some fields of OverloadCandidate in its constructor. The goal here is try to fix read of uninitialized variable (which I was not able to reproduce) llvm#93430 (comment) We should certainly try to improve the construction of `OverloadCandidate` further as it can be quite britle. (cherry picked from commit 7d787df)
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
…100318) Summary: Initialize some fields of OverloadCandidate in its constructor. The goal here is try to fix read of uninitialized variable (which I was not able to reproduce) #93430 (comment) We should certainly try to improve the construction of `OverloadCandidate` further as it can be quite britle. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250652
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
Summary: The PR #93430 introduced an assertion that did not make any sense. and caused a regression. The fix is to simply remove the assertion. No changelog. the intent is to backport this fix to clang 19. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250628
tru
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Jul 26, 2024
The PR llvm#93430 introduced an assertion that did not make any sense. and caused a regression. The fix is to simply remove the assertion. No changelog. the intent is to backport this fix to clang 19. (cherry picked from commit dd82a84)
tru
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Jul 26, 2024
…lvm#100318) Initialize some fields of OverloadCandidate in its constructor. The goal here is try to fix read of uninitialized variable (which I was not able to reproduce) llvm#93430 (comment) We should certainly try to improve the construction of `OverloadCandidate` further as it can be quite britle. (cherry picked from commit 7d787df)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement wg21.link/P2797.
Because taking the address of an explicit object member function results in a function pointer, a call expression where the id-expression is an explicit object member is made to behave consistently with that model.
This change forces clang to perform overload resolution in the presence of an id-expression of the form
(&Foo::bar)(args...), which we previously failed to do consistently.