Skip to content

Commit 4c0dc61

Browse files
SirraideAaronBallman
authored andcommitted
[Clang] [Sema] Handle placeholders in '.*' expressions (llvm#83103)
When analysing whether we should handle a binary expression as an overloaded operator call or a builtin operator, we were calling `checkPlaceholderForOverload()`, which takes care of any placeholders that are not overload sets—which would usually make sense since those need to be handled as part of overload resolution. Unfortunately, we were also doing that for `.*`, which is not overloadable, and then proceeding to create a builtin operator anyway, which would crash if the RHS happened to be an unresolved overload set (due hitting an assertion in `CreateBuiltinBinOp()`—specifically, in one of its callees—in the `.*` case that makes sure its arguments aren’t placeholders). This pr instead makes it so we check for *all* placeholders early if the operator is `.*`. It’s worth noting that, 1. In the `.*` case, we now additionally also check for *any* placeholders (not just non-overload-sets) in the LHS; this shouldn’t make a difference, however—at least I couldn’t think of a way to trigger the assertion with an overload set as the LHS of `.*`; it is worth noting that the assertion in question would also complain if the LHS happened to be of placeholder type, though. 2. There is another case in which we also don’t perform overload resolution—namely `=` if the LHS is not of class or enumeration type after handling non-overload-set placeholders—as in the `.*` case, but similarly to 1., I first couldn’t think of a way of getting this case to crash, and secondly, `CreateBuiltinBinOp()` doesn’t seem to care about placeholders in the LHS or RHS in the `=` case (from what I can tell, it, or rather one of its callees, only checks that the LHS is not a pseudo-object type, but those will have already been handled by the call to `checkPlaceholderForOverload()` by the time we get to this function), so I don’t think this case suffers from the same problem. This fixes llvm#53815. --------- Co-authored-by: Aaron Ballman <[email protected]> (cherry picked from commit d23ef9e)
1 parent a378aef commit 4c0dc61

File tree

2 files changed

+38
-5
lines changed

2 files changed

+38
-5
lines changed

clang/lib/Sema/SemaOverload.cpp

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13957,6 +13957,23 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
1395713957
CurFPFeatureOverrides());
1395813958
}
1395913959

13960+
// If this is the .* operator, which is not overloadable, just
13961+
// create a built-in binary operator.
13962+
if (Opc == BO_PtrMemD) {
13963+
auto CheckPlaceholder = [&](Expr *&Arg) {
13964+
ExprResult Res = CheckPlaceholderExpr(Arg);
13965+
if (Res.isUsable())
13966+
Arg = Res.get();
13967+
return !Res.isUsable();
13968+
};
13969+
13970+
// CreateBuiltinBinOp() doesn't like it if we tell it to create a '.*'
13971+
// expression that contains placeholders (in either the LHS or RHS).
13972+
if (CheckPlaceholder(Args[0]) || CheckPlaceholder(Args[1]))
13973+
return ExprError();
13974+
return CreateBuiltinBinOp(OpLoc, Opc, Args[0], Args[1]);
13975+
}
13976+
1396013977
// Always do placeholder-like conversions on the RHS.
1396113978
if (checkPlaceholderForOverload(*this, Args[1]))
1396213979
return ExprError();
@@ -13976,11 +13993,6 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
1397613993
if (Opc == BO_Assign && !Args[0]->getType()->isOverloadableType())
1397713994
return CreateBuiltinBinOp(OpLoc, Opc, Args[0], Args[1]);
1397813995

13979-
// If this is the .* operator, which is not overloadable, just
13980-
// create a built-in binary operator.
13981-
if (Opc == BO_PtrMemD)
13982-
return CreateBuiltinBinOp(OpLoc, Opc, Args[0], Args[1]);
13983-
1398413996
// Build the overload set.
1398513997
OverloadCandidateSet CandidateSet(OpLoc, OverloadCandidateSet::CSK_Operator,
1398613998
OverloadCandidateSet::OperatorRewriteInfo(

clang/test/SemaCXX/gh53815.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
2+
// expected-no-diagnostics
3+
4+
// Check that we don't crash due to forgetting to check for placeholders
5+
// in the RHS of '.*'.
6+
7+
template <typename Fn>
8+
static bool has_explicitly_named_overload() {
9+
return requires { Fn().*&Fn::operator(); };
10+
}
11+
12+
int main() {
13+
has_explicitly_named_overload<decltype([](auto){})>();
14+
}
15+
16+
template <typename Fn>
17+
constexpr bool has_explicitly_named_overload_2() {
18+
return requires { Fn().*&Fn::operator(); };
19+
}
20+
21+
static_assert(!has_explicitly_named_overload_2<decltype([](auto){})>());

0 commit comments

Comments
 (0)