Skip to content

Commit 01682af

Browse files
DougGregorDougGregor
committed
Revert "[Type checker] Be more rigorous about extracting argument labels from calls."
This reverts commit 93dac8f.
1 parent 2887889 commit 01682af

File tree

7 files changed

+91
-110
lines changed

7 files changed

+91
-110
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/Sema/CSGen.cpp

Lines changed: 77 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1597,12 +1597,6 @@ namespace {
15971597

15981598
// If there is an argument, apply it.
15991599
if (auto arg = expr->getArgument()) {
1600-
// Identify and record the argument labels for this call.
1601-
if (auto argumentLabels = getArgumentLabelsForCallArgument(arg)) {
1602-
CS.CalleeArgumentLabels[CS.getConstraintLocator(expr)] =
1603-
*argumentLabels;
1604-
}
1605-
16061600
// The result type of the function must be convertible to the base type.
16071601
// TODO: we definitely want this to include ImplicitlyUnwrappedOptional; does it
16081602
// need to include everything else in the world?
@@ -2435,52 +2429,11 @@ namespace {
24352429
return expr->getType();
24362430
}
24372431

2438-
using ArgumentLabelState = ConstraintSystem::ArgumentLabelState;
2439-
2440-
/// Extract argument labels from the given call argument.
2441-
Optional<ArgumentLabelState> getArgumentLabelsForCallArgument(Expr *arg) {
2442-
// Parentheses.
2443-
if (auto paren = dyn_cast<ParenExpr>(arg)) {
2444-
return ArgumentLabelState{
2445-
CS.allocateCopy(llvm::makeArrayRef(Identifier())),
2446-
paren->hasTrailingClosure() };
2447-
}
2448-
2449-
// FIXME: Hack because not all CallExprs come in with
2450-
// ParenExpr/TupleExpr arguments.
2451-
if (!isa<TupleExpr>(arg)) return None;
2452-
2453-
// Tuples.
2454-
auto tuple = cast<TupleExpr>(arg);
2455-
2456-
// If we have element names, use 'em.
2457-
if (tuple->hasElementNames()) {
2458-
return ArgumentLabelState{tuple->getElementNames(),
2459-
tuple->hasTrailingClosure()};
2460-
}
2461-
2462-
// Otherwise, there are no argument labels.
2463-
llvm::SmallVector<Identifier, 4> names(tuple->getNumElements(),
2464-
Identifier());
2465-
return ArgumentLabelState{CS.allocateCopy(names),
2466-
tuple->hasTrailingClosure()};
2467-
}
2468-
24692432
Type visitApplyExpr(ApplyExpr *expr) {
24702433
Type outputTy;
24712434

24722435
auto fnExpr = expr->getFn();
2473-
2474-
// Identify and record the argument labels for a call.
2475-
if (auto call = dyn_cast<CallExpr>(expr)) {
2476-
if (auto argumentLabels =
2477-
getArgumentLabelsForCallArgument(expr->getArg())) {
2478-
auto callee = call->getDirectCallee();
2479-
CS.CalleeArgumentLabels[CS.getConstraintLocator(callee)] =
2480-
*argumentLabels;
2481-
}
2482-
}
2483-
2436+
24842437
if (isa<DeclRefExpr>(fnExpr)) {
24852438
if (auto fnType = fnExpr->getType()->getAs<AnyFunctionType>()) {
24862439
outputTy = fnType->getResult();
@@ -2583,7 +2536,7 @@ namespace {
25832536
FunctionType::ExtInfo extInfo;
25842537
if (isa<ClosureExpr>(fnExpr->getSemanticsProvidingExpr()))
25852538
extInfo = extInfo.withNoEscape();
2586-
2539+
25872540
auto funcTy = FunctionType::get(expr->getArg()->getType(), outputTy,
25882541
extInfo);
25892542

@@ -3046,12 +2999,87 @@ namespace {
30462999
/// \brief Ignore declarations.
30473000
bool walkToDeclPre(Decl *decl) override { return false; }
30483001
};
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+
30493074
} // end anonymous namespace
30503075

30513076
Expr *ConstraintSystem::generateConstraints(Expr *expr) {
30523077
// Remove implicit conversions from the expression.
30533078
expr = expr->walk(SanitizeExpr(getTypeChecker()));
30543079

3080+
// Walk the expression to associate labeled arguments.
3081+
expr->walk(ArgumentLabelWalker(*this, expr));
3082+
30553083
// Walk the expression, generating constraints.
30563084
ConstraintGenerator cg(*this);
30573085
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/ConstraintSystem.h

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,25 +1042,16 @@ class ConstraintSystem {
10421042
/// we're exploring.
10431043
SolverState *solverState = nullptr;
10441044

1045-
/// Describes argument labels as they occur in a call.
10461045
struct ArgumentLabelState {
1047-
/// The actual argument labels provided at the call site.
1048-
///
1049-
/// The contents of this array are only guaranteed to live until
1050-
/// the constraint system is destroyed.
10511046
ArrayRef<Identifier> Labels;
1052-
1053-
/// Whether the last of the arguments was written as a trailing
1054-
/// closure. It will have an empty Identifier().
10551047
bool HasTrailingClosure;
10561048
};
10571049

1058-
/// The argument labels that are associated with a given callee.
1059-
///
1060-
/// The argument labels for a particular callee are recorded at the
1061-
/// time of constraint generation based on the syntactic call
1062-
/// argument.
1063-
llvm::DenseMap<ConstraintLocator *, ArgumentLabelState> CalleeArgumentLabels;
1050+
/// A mapping from the constraint locators for references to various
1051+
/// names (e.g., member references, normal name references, possible
1052+
/// constructions) to the argument labels provided in the call to
1053+
/// that locator.
1054+
llvm::DenseMap<ConstraintLocator *, ArgumentLabelState> ArgumentLabels;
10641055

10651056
/// FIXME: This is a workaround for the way we perform protocol
10661057
/// conformance checking for generic requirements, where we re-use

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2830,16 +2830,14 @@ class ObjCSelectorWalker : public ASTWalker {
28302830
bool hadParens = false;
28312831
auto lookThroughParens = [&](Expr *arg, bool outermost) -> Expr * {
28322832
if (auto parenExpr = dyn_cast<ParenExpr>(arg)) {
2833-
if (!outermost && !parenExpr->isImplicit()) {
2833+
if (!outermost) {
28342834
hadParens = true;
28352835
return parenExpr->getSubExpr()->getSemanticsProvidingExpr();
28362836
}
28372837

28382838
arg = parenExpr->getSubExpr();
28392839
if (auto innerParenExpr = dyn_cast<ParenExpr>(arg)) {
2840-
if (!innerParenExpr->isImplicit())
2841-
hadParens = true;
2842-
2840+
hadParens = true;
28432841
arg = innerParenExpr->getSubExpr();
28442842
}
28452843
}

‎lib/Sema/TypeCheckConstraints.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -792,8 +792,10 @@ namespace {
792792
continue;
793793
}
794794

795-
// Look through force-value, and 'try' expressions.
796-
if (isa<ForceValueExpr>(ancestor) || isa<AnyTryExpr>(ancestor)) {
795+
// Look through identity, force-value, and 'try' expressions.
796+
if (isa<IdentityExpr>(ancestor) ||
797+
isa<ForceValueExpr>(ancestor) ||
798+
isa<AnyTryExpr>(ancestor)) {
797799
if (target)
798800
target = ancestor;
799801
continue;

0 commit comments

Comments
 (0)