Skip to content

Commit ee85891

Browse files
DougGregorDougGregor
committed
Revert "[Type checker] Be more rigorous about extracting argument labels from calls."
This reverts commit 3753d77. It's causing some type-inference problems I need to investigate.
1 parent 2e7ae3d commit ee85891

File tree

12 files changed

+112
-157
lines changed

12 files changed

+112
-157
lines changed

‎include/swift/AST/Expr.h

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,9 +1621,6 @@ class ParenExpr : public IdentityExpr {
16211621
/// \brief Whether this expression has a trailing closure as its argument.
16221622
bool hasTrailingClosure() const { return HasTrailingClosure; }
16231623

1624-
/// Create a new, implicit parenthesized expression.
1625-
static ParenExpr *createImplicit(ASTContext &ctx, Expr *expr);
1626-
16271624
static bool classof(const Expr *E) { return E->getKind() == ExprKind::Paren; }
16281625
};
16291626

@@ -3198,15 +3195,7 @@ class CallExpr : public ApplyExpr {
31983195
SourceLoc FnLoc = getFn()->getLoc();
31993196
return FnLoc.isValid() ? FnLoc : getArg()->getLoc();
32003197
}
3201-
3202-
/// Retrieve the expression that direct represents the callee.
3203-
///
3204-
/// The "direct" callee is the expression representing the callee
3205-
/// after looking through top-level constructs that don't affect the
3206-
/// identity of the callee, e.g., extra parentheses, optional
3207-
/// unwrapping (?)/forcing (!), etc.
3208-
Expr *getDirectCallee() const;
3209-
3198+
32103199
static bool classof(const Expr *E) { return E->getKind() == ExprKind::Call; }
32113200
};
32123201

‎lib/AST/Expr.cpp

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -922,14 +922,6 @@ SequenceExpr *SequenceExpr::create(ASTContext &ctx, ArrayRef<Expr*> elements) {
922922
return ::new(Buffer) SequenceExpr(elements);
923923
}
924924

925-
ParenExpr *ParenExpr::createImplicit(ASTContext &ctx, Expr *expr) {
926-
ParenExpr *result = new (ctx) ParenExpr(SourceLoc(), expr, SourceLoc(),
927-
/*hasTrailingClosure=*/false,
928-
expr->getType());
929-
result->setImplicit();
930-
return result;
931-
}
932-
933925
SourceLoc TupleExpr::getStartLoc() const {
934926
if (LParenLoc.isValid()) return LParenLoc;
935927
if (getNumElements() == 0) return SourceLoc();
@@ -1041,25 +1033,6 @@ ValueDecl *ApplyExpr::getCalledValue() const {
10411033
return ::getCalledValue(Fn);
10421034
}
10431035

1044-
Expr *CallExpr::getDirectCallee() const {
1045-
auto fn = getFn();
1046-
while (true) {
1047-
fn = fn->getSemanticsProvidingExpr();
1048-
1049-
if (auto force = dyn_cast<ForceValueExpr>(fn)) {
1050-
fn = force->getSubExpr();
1051-
continue;
1052-
}
1053-
1054-
if (auto bind = dyn_cast<BindOptionalExpr>(fn)) {
1055-
fn = bind->getSubExpr();
1056-
continue;
1057-
}
1058-
1059-
return fn;
1060-
}
1061-
}
1062-
10631036
RebindSelfInConstructorExpr::RebindSelfInConstructorExpr(Expr *SubExpr,
10641037
VarDecl *Self)
10651038
: Expr(ExprKind::RebindSelfInConstructor, /*Implicit=*/true,

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -380,11 +380,8 @@ makeEnumRawValueConstructor(ClangImporter::Implementation &Impl,
380380
return ctorDecl;
381381

382382
auto selfRef = new (C) DeclRefExpr(selfDecl, DeclNameLoc(), /*implicit*/true);
383-
Expr *paramRef = new (C) DeclRefExpr(param, DeclNameLoc(),
384-
/*implicit*/ true);
385-
paramRef = ParenExpr::createImplicit(C, paramRef);
386-
paramRef->setImplicit();
387-
383+
auto paramRef = new (C) DeclRefExpr(param, DeclNameLoc(),
384+
/*implicit*/ true);
388385
auto reinterpretCast
389386
= cast<FuncDecl>(getBuiltinValueDecl(C,C.getIdentifier("reinterpretCast")));
390387
auto reinterpretCastRef
@@ -443,11 +440,7 @@ static FuncDecl *makeEnumRawValueGetter(ClangImporter::Implementation &Impl,
443440
if (Impl.hasFinishedTypeChecking())
444441
return getterDecl;
445442

446-
Expr *selfRef = new (C) DeclRefExpr(selfDecl, DeclNameLoc(),
447-
/*implicit*/true);
448-
selfRef = ParenExpr::createImplicit(C, selfRef);
449-
selfRef->setImplicit();
450-
443+
auto selfRef = new (C) DeclRefExpr(selfDecl, DeclNameLoc(), /*implicit*/true);
451444
auto reinterpretCast
452445
= cast<FuncDecl>(getBuiltinValueDecl(C, C.getIdentifier("reinterpretCast")));
453446
auto reinterpretCastRef
@@ -623,11 +616,8 @@ makeUnionFieldAccessors(ClangImporter::Implementation &Impl,
623616
{
624617
auto selfDecl = getterDecl->getImplicitSelfDecl();
625618

626-
Expr *selfRef = new (C) DeclRefExpr(selfDecl, DeclNameLoc(),
627-
/*implicit*/ true);
628-
selfRef = ParenExpr::createImplicit(C, selfRef);
629-
selfRef->setImplicit();
630-
619+
auto selfRef = new (C) DeclRefExpr(selfDecl, DeclNameLoc(),
620+
/*implicit*/ true);
631621
auto reinterpretCast = cast<FuncDecl>(getBuiltinValueDecl(
632622
C, C.getIdentifier("reinterpretCast")));
633623
auto reinterpretCastRef
@@ -648,10 +638,8 @@ makeUnionFieldAccessors(ClangImporter::Implementation &Impl,
648638

649639
auto inoutSelfRef = new (C) DeclRefExpr(inoutSelfDecl, DeclNameLoc(),
650640
/*implicit*/ true);
651-
Expr *inoutSelf = new (C) InOutExpr(SourceLoc(), inoutSelfRef,
641+
auto inoutSelf = new (C) InOutExpr(SourceLoc(), inoutSelfRef,
652642
InOutType::get(importedUnionDecl->getType()), /*implicit*/ true);
653-
inoutSelf = ParenExpr::createImplicit(C, inoutSelf);
654-
inoutSelf->setImplicit();
655643

656644
auto newValueDecl = setterDecl->getParameterList(1)->get(0);
657645

‎lib/Sema/CSApply.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5254,8 +5254,6 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
52545254
Expr *fnRef = new (tc.Context) DeclRefExpr(fnDeclRef,
52555255
DeclNameLoc(expr->getLoc()),
52565256
/*Implicit=*/true);
5257-
expr = ParenExpr::createImplicit(tc.Context, expr);
5258-
52595257
fnRef->setType(fn->getInterfaceType());
52605258
Expr *call = new (tc.Context) CallExpr(fnRef, expr,
52615259
/*implicit*/ true);
@@ -6595,8 +6593,6 @@ Expr *TypeChecker::callWitness(Expr *base, DeclContext *dc,
65956593
argumentNamesMatch(arguments[0],
65966594
witness->getFullName().getArgumentNames()))) {
65976595
arg = arguments[0];
6598-
if (!isa<TupleExpr>(arg) && !isa<ParenExpr>(arg))
6599-
arg = ParenExpr::createImplicit(Context, arg);
66006596
} else {
66016597
SmallVector<TupleTypeElt, 4> elementTypes;
66026598
auto names = witness->getFullName().getArgumentNames();
@@ -6844,7 +6840,6 @@ Expr *Solution::convertOptionalToBool(Expr *expr,
68446840
fnRef->setType(fn->getInterfaceType().subst(
68456841
constraintSystem->DC->getParentModule(), subMap, None));
68466842

6847-
expr = ParenExpr::createImplicit(ctx, expr);
68486843
Expr *call = new (ctx) CallExpr(fnRef, expr, /*Implicit=*/true);
68496844

68506845
bool failed = tc.typeCheckExpressionShallow(call, cs.DC);

‎lib/Sema/CSGen.cpp

Lines changed: 77 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2429,69 +2429,11 @@ namespace {
24292429
return expr->getType();
24302430
}
24312431

2432-
using ArgumentLabelState = ConstraintSystem::ArgumentLabelState;
2433-
2434-
/// Extract argument labels from the given call argument.
2435-
ArgumentLabelState getArgumentLabelsForCallArgument(Expr *arg) {
2436-
// Parentheses.
2437-
if (auto paren = dyn_cast<ParenExpr>(arg)) {
2438-
return ArgumentLabelState{
2439-
CS.allocateCopy(llvm::makeArrayRef(Identifier())),
2440-
paren->hasTrailingClosure() };
2441-
}
2442-
2443-
// Type representations.
2444-
// FIXME: This should never happen, but
2445-
// PreCheckExpression::simplifyTypeExpr() tends to pull in the
2446-
// parentheses of call arguments overly eagerly, and the fix
2447-
// implies enforcing ".self" more rigorously than we do now,
2448-
// which is at odds with the expected direction of SE-0090.
2449-
if (auto typeExpr = dyn_cast<TypeExpr>(arg)) {
2450-
auto typeRepr = typeExpr->getTypeRepr();
2451-
auto tupleType = cast<TupleTypeRepr>(typeRepr);
2452-
2453-
// Collect the names.
2454-
SmallVector<Identifier, 4> names;
2455-
names.reserve(tupleType->getElements().size());
2456-
for (auto elt : tupleType->getElements()) {
2457-
if (auto named = dyn_cast<NamedTypeRepr>(elt))
2458-
names.push_back(named->getName());
2459-
else
2460-
names.push_back(Identifier());
2461-
}
2462-
2463-
return ArgumentLabelState{CS.allocateCopy(names), false};
2464-
}
2465-
2466-
// Tuples.
2467-
auto tuple = cast<TupleExpr>(arg);
2468-
2469-
// If we have element names, use 'em.
2470-
if (tuple->hasElementNames()) {
2471-
return ArgumentLabelState{tuple->getElementNames(),
2472-
tuple->hasTrailingClosure()};
2473-
}
2474-
2475-
// Otherwise, there are no argument labels.
2476-
llvm::SmallVector<Identifier, 4> names(tuple->getNumElements(),
2477-
Identifier());
2478-
return ArgumentLabelState{CS.allocateCopy(names),
2479-
tuple->hasTrailingClosure()};
2480-
}
2481-
24822432
Type visitApplyExpr(ApplyExpr *expr) {
24832433
Type outputTy;
24842434

24852435
auto fnExpr = expr->getFn();
2486-
2487-
// Identify and record the argument labels for a call.
2488-
if (auto call = dyn_cast<CallExpr>(expr)) {
2489-
auto argumentLabels = getArgumentLabelsForCallArgument(expr->getArg());
2490-
auto callee = call->getDirectCallee();
2491-
CS.CalleeArgumentLabels[CS.getConstraintLocator(callee)] =
2492-
argumentLabels;
2493-
}
2494-
2436+
24952437
if (isa<DeclRefExpr>(fnExpr)) {
24962438
if (auto fnType = fnExpr->getType()->getAs<AnyFunctionType>()) {
24972439
outputTy = fnType->getResult();
@@ -2594,7 +2536,7 @@ namespace {
25942536
FunctionType::ExtInfo extInfo;
25952537
if (isa<ClosureExpr>(fnExpr->getSemanticsProvidingExpr()))
25962538
extInfo = extInfo.withNoEscape();
2597-
2539+
25982540
auto funcTy = FunctionType::get(expr->getArg()->getType(), outputTy,
25992541
extInfo);
26002542

@@ -3057,12 +2999,87 @@ namespace {
30572999
/// \brief Ignore declarations.
30583000
bool walkToDeclPre(Decl *decl) override { return false; }
30593001
};
3002+
3003+
/// AST walker that records the keyword arguments provided at each
3004+
/// call site.
3005+
class ArgumentLabelWalker : public ASTWalker {
3006+
ConstraintSystem &CS;
3007+
llvm::DenseMap<Expr *, Expr *> ParentMap;
3008+
3009+
public:
3010+
ArgumentLabelWalker(ConstraintSystem &cs, Expr *expr)
3011+
: CS(cs), ParentMap(expr->getParentMap()) { }
3012+
3013+
using State = ConstraintSystem::ArgumentLabelState;
3014+
3015+
void associateArgumentLabels(Expr *arg, State labels,
3016+
bool labelsArePermanent) {
3017+
// Our parent must be a call.
3018+
auto call = dyn_cast_or_null<CallExpr>(ParentMap[arg]);
3019+
if (!call)
3020+
return;
3021+
3022+
// We must have originated at the call argument.
3023+
if (arg != call->getArg())
3024+
return;
3025+
3026+
// Dig out the function, looking through, parentheses, ?, and !.
3027+
auto fn = call->getFn();
3028+
do {
3029+
fn = fn->getSemanticsProvidingExpr();
3030+
3031+
if (auto force = dyn_cast<ForceValueExpr>(fn)) {
3032+
fn = force->getSubExpr();
3033+
continue;
3034+
}
3035+
3036+
if (auto bind = dyn_cast<BindOptionalExpr>(fn)) {
3037+
fn = bind->getSubExpr();
3038+
continue;
3039+
}
3040+
3041+
break;
3042+
} while (true);
3043+
3044+
// Record the labels.
3045+
if (!labelsArePermanent)
3046+
labels.Labels = CS.allocateCopy(labels.Labels);
3047+
CS.ArgumentLabels[CS.getConstraintLocator(fn)] = labels;
3048+
}
3049+
3050+
std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
3051+
if (auto tuple = dyn_cast<TupleExpr>(expr)) {
3052+
if (tuple->hasElementNames())
3053+
associateArgumentLabels(expr,
3054+
{ tuple->getElementNames(),
3055+
tuple->hasTrailingClosure() },
3056+
/*labelsArePermanent*/ true);
3057+
else {
3058+
llvm::SmallVector<Identifier, 4> names(tuple->getNumElements(),
3059+
Identifier());
3060+
associateArgumentLabels(expr, { names, tuple->hasTrailingClosure() },
3061+
/*labelsArePermanent*/ false);
3062+
}
3063+
} else if (auto paren = dyn_cast<ParenExpr>(expr)) {
3064+
associateArgumentLabels(paren,
3065+
{ { Identifier() },
3066+
paren->hasTrailingClosure() },
3067+
/*labelsArePermanent*/ false);
3068+
}
3069+
3070+
return { true, expr };
3071+
}
3072+
};
3073+
30603074
} // end anonymous namespace
30613075

30623076
Expr *ConstraintSystem::generateConstraints(Expr *expr) {
30633077
// Remove implicit conversions from the expression.
30643078
expr = expr->walk(SanitizeExpr(getTypeChecker()));
30653079

3080+
// Walk the expression to associate labeled arguments.
3081+
expr->walk(ArgumentLabelWalker(*this, expr));
3082+
30663083
// Walk the expression, generating constraints.
30673084
ConstraintGenerator cg(*this);
30683085
ConstraintWalker cw(cg);

‎lib/Sema/CSSimplify.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2612,8 +2612,8 @@ getArgumentLabels(ConstraintSystem &cs, ConstraintLocatorBuilder locator) {
26122612
if (!parts.empty())
26132613
return None;
26142614

2615-
auto known = cs.CalleeArgumentLabels.find(cs.getConstraintLocator(anchor));
2616-
if (known == cs.CalleeArgumentLabels.end())
2615+
auto known = cs.ArgumentLabels.find(cs.getConstraintLocator(anchor));
2616+
if (known == cs.ArgumentLabels.end())
26172617
return None;
26182618

26192619
return known->second;

‎lib/Sema/CodeSynthesis.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -359,12 +359,16 @@ static Expr *buildArgumentForwardingExpr(ArrayRef<ParamDecl*> params,
359359

360360
// A single unlabelled value is not a tuple.
361361
if (args.size() == 1 && labels[0].empty())
362-
return ParenExpr::createImplicit(ctx, args[0]);
362+
return args[0];
363363

364364
return TupleExpr::create(ctx, SourceLoc(), args, labels, labelLocs,
365365
SourceLoc(), false, IsImplicit);
366366
}
367367

368+
369+
370+
371+
368372
/// Build a reference to the subscript index variables for this subscript
369373
/// accessor.
370374
static Expr *buildSubscriptIndexReference(ASTContext &ctx, FuncDecl *accessor) {
@@ -915,10 +919,8 @@ void swift::synthesizeObservingAccessors(VarDecl *VD, TypeChecker &TC) {
915919
// (call_expr (decl_ref_expr(willSet)), (declrefexpr(value)))
916920
if (auto willSet = VD->getWillSetFunc()) {
917921
Expr *Callee = new (Ctx) DeclRefExpr(willSet, DeclNameLoc(), /*imp*/true);
918-
Expr *ValueDRE = new (Ctx) DeclRefExpr(ValueDecl, DeclNameLoc(),
922+
auto *ValueDRE = new (Ctx) DeclRefExpr(ValueDecl, DeclNameLoc(),
919923
/*imp*/true);
920-
ValueDRE = ParenExpr::createImplicit(Ctx, ValueDRE);
921-
922924
if (SelfDecl) {
923925
auto *SelfDRE = new (Ctx) DeclRefExpr(SelfDecl, DeclNameLoc(),
924926
/*imp*/true);
@@ -943,10 +945,8 @@ void swift::synthesizeObservingAccessors(VarDecl *VD, TypeChecker &TC) {
943945
// or:
944946
// (call_expr (decl_ref_expr(didSet)), (decl_ref_expr(tmp)))
945947
if (auto didSet = VD->getDidSetFunc()) {
946-
Expr *OldValueExpr = new (Ctx) DeclRefExpr(OldValue, DeclNameLoc(),
948+
auto *OldValueExpr = new (Ctx) DeclRefExpr(OldValue, DeclNameLoc(),
947949
/*impl*/true);
948-
OldValueExpr = ParenExpr::createImplicit(Ctx, OldValueExpr);
949-
950950
Expr *Callee = new (Ctx) DeclRefExpr(didSet, DeclNameLoc(), /*imp*/true);
951951
if (SelfDecl) {
952952
auto *SelfDRE = new (Ctx) DeclRefExpr(SelfDecl, DeclNameLoc(),
@@ -1231,7 +1231,7 @@ void TypeChecker::completePropertyBehaviorStorage(VarDecl *VD,
12311231
// Type-check the expression.
12321232
typeCheckExpression(InitValue, DC);
12331233

1234-
InitStorageParam = ParenExpr::createImplicit(Context, InitValue);
1234+
InitStorageParam = InitValue;
12351235
} else {
12361236
InitStorageParam = TupleExpr::createEmpty(Context,
12371237
SourceLoc(), SourceLoc(),

0 commit comments

Comments
 (0)