Skip to content

Commit 334b76a

Browse files
authored
Merge pull request #24084 from brentdax/whos-that-key-path-component
Improve `-debug-constraints` output for key path components
2 parents 9295074 + 760d4be commit 334b76a

File tree

9 files changed

+162
-72
lines changed

9 files changed

+162
-72
lines changed

include/swift/AST/Expr.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ namespace swift {
6060
class ParameterList;
6161
class EnumElementDecl;
6262
class CallExpr;
63+
class KeyPathExpr;
6364

6465
enum class ExprKind : uint8_t {
6566
#define EXPR(Id, Parent) Id,
@@ -539,6 +540,7 @@ class alignas(8) Expr {
539540
void dump(raw_ostream &OS, unsigned Indent = 0) const;
540541
void dump(raw_ostream &OS, llvm::function_ref<Type(const Expr *)> getType,
541542
llvm::function_ref<Type(const TypeLoc &)> getTypeOfTypeLoc,
543+
llvm::function_ref<Type(const KeyPathExpr *E, unsigned index)> getTypeOfKeyPathComponent,
542544
unsigned Indent = 0) const;
543545

544546
void print(ASTPrinter &Printer, const PrintOptions &Opts) const;

lib/AST/ASTDumper.cpp

Lines changed: 56 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1727,14 +1727,17 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
17271727
raw_ostream &OS;
17281728
llvm::function_ref<Type(const Expr *)> GetTypeOfExpr;
17291729
llvm::function_ref<Type(const TypeLoc &)> GetTypeOfTypeLoc;
1730+
llvm::function_ref<Type(const KeyPathExpr *E, unsigned index)> GetTypeOfKeyPathComponent;
17301731
unsigned Indent;
17311732

17321733
PrintExpr(raw_ostream &os,
17331734
llvm::function_ref<Type(const Expr *)> getTypeOfExpr,
17341735
llvm::function_ref<Type(const TypeLoc &)> getTypeOfTypeLoc,
1736+
llvm::function_ref<Type(const KeyPathExpr *E, unsigned index)> getTypeOfKeyPathComponent,
17351737
unsigned indent)
17361738
: OS(os), GetTypeOfExpr(getTypeOfExpr),
1737-
GetTypeOfTypeLoc(getTypeOfTypeLoc), Indent(indent) {}
1739+
GetTypeOfTypeLoc(getTypeOfTypeLoc),
1740+
GetTypeOfKeyPathComponent(getTypeOfKeyPathComponent), Indent(indent) {}
17381741

17391742
void printRec(Expr *E) {
17401743
Indent += 2;
@@ -2612,83 +2615,94 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
26122615
printCommon(E, "keypath_expr");
26132616
if (E->isObjC())
26142617
OS << " objc";
2615-
for (auto &component : E->getComponents()) {
2618+
2619+
OS << '\n';
2620+
Indent += 2;
2621+
OS.indent(Indent);
2622+
PrintWithColorRAII(OS, ParenthesisColor) << '(';
2623+
PrintWithColorRAII(OS, ExprColor) << "components";
2624+
OS.indent(Indent + 2);
2625+
for (unsigned i : indices(E->getComponents())) {
2626+
auto &component = E->getComponents()[i];
26162627
OS << '\n';
26172628
OS.indent(Indent + 2);
2618-
OS << "(component=";
2629+
PrintWithColorRAII(OS, ParenthesisColor) << '(';
26192630
switch (component.getKind()) {
26202631
case KeyPathExpr::Component::Kind::Invalid:
2621-
OS << "invalid ";
2632+
PrintWithColorRAII(OS, ASTNodeColor) << "invalid";
26222633
break;
26232634

26242635
case KeyPathExpr::Component::Kind::OptionalChain:
2625-
OS << "optional_chain ";
2636+
PrintWithColorRAII(OS, ASTNodeColor) << "optional_chain";
26262637
break;
26272638

26282639
case KeyPathExpr::Component::Kind::OptionalForce:
2629-
OS << "optional_force ";
2640+
PrintWithColorRAII(OS, ASTNodeColor) << "optional_force";
26302641
break;
26312642

26322643
case KeyPathExpr::Component::Kind::OptionalWrap:
2633-
OS << "optional_wrap ";
2644+
PrintWithColorRAII(OS, ASTNodeColor) << "optional_wrap";
26342645
break;
26352646

26362647
case KeyPathExpr::Component::Kind::Property:
2637-
OS << "property ";
2648+
PrintWithColorRAII(OS, ASTNodeColor) << "property";
2649+
PrintWithColorRAII(OS, DeclColor) << " decl=";
26382650
printDeclRef(component.getDeclRef());
2639-
OS << " ";
26402651
break;
26412652

26422653
case KeyPathExpr::Component::Kind::Subscript:
2643-
OS << "subscript ";
2654+
PrintWithColorRAII(OS, ASTNodeColor) << "subscript";
2655+
PrintWithColorRAII(OS, DeclColor) << " decl='";
26442656
printDeclRef(component.getDeclRef());
2645-
OS << '\n';
2646-
component.getIndexExpr()->dump(OS, Indent + 4);
2647-
OS.indent(Indent + 4);
2657+
PrintWithColorRAII(OS, DeclColor) << "'";
26482658
break;
26492659

26502660
case KeyPathExpr::Component::Kind::UnresolvedProperty:
2651-
OS << "unresolved_property ";
2652-
component.getUnresolvedDeclName().print(OS);
2653-
OS << " ";
2661+
PrintWithColorRAII(OS, ASTNodeColor) << "unresolved_property";
2662+
PrintWithColorRAII(OS, IdentifierColor)
2663+
<< " decl_name='" << component.getUnresolvedDeclName() << "'";
26542664
break;
26552665

26562666
case KeyPathExpr::Component::Kind::UnresolvedSubscript:
2657-
OS << "unresolved_subscript";
2658-
OS << '\n';
2659-
component.getIndexExpr()->dump(OS, Indent + 4);
2660-
OS.indent(Indent + 4);
2667+
PrintWithColorRAII(OS, ASTNodeColor) << "unresolved_subscript";
2668+
printArgumentLabels(component.getSubscriptLabels());
26612669
break;
26622670
case KeyPathExpr::Component::Kind::Identity:
2663-
OS << "identity";
2664-
OS << '\n';
2671+
PrintWithColorRAII(OS, ASTNodeColor) << "identity";
26652672
break;
2673+
26662674
case KeyPathExpr::Component::Kind::TupleElement:
2667-
OS << "tuple_element ";
2668-
OS << "#" << component.getTupleIndex();
2669-
OS << " ";
2675+
PrintWithColorRAII(OS, ASTNodeColor) << "tuple_element ";
2676+
PrintWithColorRAII(OS, DiscriminatorColor)
2677+
<< "#" << component.getTupleIndex();
26702678
break;
26712679
}
2672-
OS << "type=";
2673-
component.getComponentType().print(OS);
2680+
PrintWithColorRAII(OS, TypeColor)
2681+
<< " type='" << GetTypeOfKeyPathComponent(E, i) << "'";
2682+
if (auto indexExpr = component.getIndexExpr()) {
2683+
OS << '\n';
2684+
Indent += 2;
2685+
printRec(indexExpr);
2686+
Indent -= 2;
2687+
}
26742688
PrintWithColorRAII(OS, ParenthesisColor) << ')';
26752689
}
2690+
2691+
PrintWithColorRAII(OS, ParenthesisColor) << ')';
2692+
Indent -= 2;
2693+
26762694
if (auto stringLiteral = E->getObjCStringLiteralExpr()) {
26772695
OS << '\n';
2678-
printRec(stringLiteral);
2696+
printRecLabeled(stringLiteral, "objc_string_literal");
26792697
}
26802698
if (!E->isObjC()) {
2681-
OS << "\n";
26822699
if (auto root = E->getParsedRoot()) {
2683-
printRec(root);
2684-
} else {
2685-
OS.indent(Indent + 2) << "<<null>>";
2700+
OS << "\n";
2701+
printRecLabeled(root, "parsed_root");
26862702
}
2687-
OS << "\n";
26882703
if (auto path = E->getParsedPath()) {
2689-
printRec(path);
2690-
} else {
2691-
OS.indent(Indent + 2) << "<<null>>";
2704+
OS << "\n";
2705+
printRecLabeled(path, "parsed_path");
26922706
}
26932707
}
26942708
PrintWithColorRAII(OS, ParenthesisColor) << ')';
@@ -2723,8 +2737,9 @@ void Expr::dump() const {
27232737
void Expr::dump(raw_ostream &OS,
27242738
llvm::function_ref<Type(const Expr *)> getTypeOfExpr,
27252739
llvm::function_ref<Type(const TypeLoc &)> getTypeOfTypeLoc,
2740+
llvm::function_ref<Type(const KeyPathExpr *E, unsigned index)> getTypeOfKeyPathComponent,
27262741
unsigned Indent) const {
2727-
PrintExpr(OS, getTypeOfExpr, getTypeOfTypeLoc, Indent)
2742+
PrintExpr(OS, getTypeOfExpr, getTypeOfTypeLoc, getTypeOfKeyPathComponent, Indent)
27282743
.visit(const_cast<Expr *>(this));
27292744
}
27302745

@@ -2733,7 +2748,10 @@ void Expr::dump(raw_ostream &OS, unsigned Indent) const {
27332748
auto getTypeOfTypeLoc = [](const TypeLoc &TL) -> Type {
27342749
return TL.getType();
27352750
};
2736-
dump(OS, getTypeOfExpr, getTypeOfTypeLoc, Indent);
2751+
auto getTypeOfKeyPathComponent = [](const KeyPathExpr *E, unsigned index) -> Type {
2752+
return E->getComponents()[index].getComponentType();
2753+
};
2754+
dump(OS, getTypeOfExpr, getTypeOfTypeLoc, getTypeOfKeyPathComponent, Indent);
27372755
}
27382756

27392757
void Expr::print(ASTPrinter &Printer, const PrintOptions &Opts) const {

lib/Sema/CSApply.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1698,9 +1698,12 @@ namespace {
16981698
}
16991699

17001700
assert(componentExpr);
1701-
componentExpr->setType(simplifyType(cs.getType(anchor)));
1701+
Type ty = simplifyType(cs.getType(anchor));
1702+
componentExpr->setType(ty);
17021703
cs.cacheType(componentExpr);
17031704

1705+
cs.setType(keyPath, 0, ty);
1706+
17041707
keyPath->setParsedPath(componentExpr);
17051708
keyPath->resolveComponents(ctx, {component});
17061709
return keyPath;
@@ -4358,6 +4361,13 @@ namespace {
43584361
Type baseTy = keyPathTy->getGenericArgs()[0];
43594362
Type leafTy = keyPathTy->getGenericArgs()[1];
43604363

4364+
// Updates the constraint system with the type of the last resolved
4365+
// component. We do it this way because we sometimes insert new
4366+
// components.
4367+
auto updateCSWithResolvedComponent = [&]() {
4368+
cs.setType(E, resolvedComponents.size() - 1, baseTy);
4369+
};
4370+
43614371
for (unsigned i : indices(E->getComponents())) {
43624372
auto &origComponent = E->getMutableComponents()[i];
43634373

@@ -4434,6 +4444,7 @@ namespace {
44344444
resolvedComponents.push_back(component);
44354445

44364446
if (shouldForceUnwrapResult(foundDecl->choice, locator)) {
4447+
updateCSWithResolvedComponent();
44374448
auto objectTy = getObjectType(baseTy);
44384449
auto loc = origComponent.getLoc();
44394450
component = KeyPathExpr::Component::forOptionalForce(objectTy, loc);
@@ -4461,6 +4472,7 @@ namespace {
44614472
resolvedComponents.push_back(component);
44624473

44634474
if (shouldForceUnwrapResult(foundDecl->choice, locator)) {
4475+
updateCSWithResolvedComponent();
44644476
auto objectTy = getObjectType(baseTy);
44654477
auto loc = origComponent.getLoc();
44664478
component = KeyPathExpr::Component::forOptionalForce(objectTy, loc);
@@ -4512,6 +4524,9 @@ namespace {
45124524
case KeyPathExpr::Component::Kind::TupleElement:
45134525
llvm_unreachable("already resolved");
45144526
}
4527+
4528+
// By now, "baseTy" is the result type of this component.
4529+
updateCSWithResolvedComponent();
45154530
}
45164531

45174532
// Wrap a non-optional result if there was chaining involved.
@@ -4524,6 +4539,7 @@ namespace {
45244539
auto component = KeyPathExpr::Component::forOptionalWrap(leafTy);
45254540
resolvedComponents.push_back(component);
45264541
baseTy = leafTy;
4542+
updateCSWithResolvedComponent();
45274543
}
45284544
E->resolveComponents(cs.getASTContext(), resolvedComponents);
45294545

lib/Sema/CSGen.cpp

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2974,6 +2974,11 @@ namespace {
29742974

29752975
for (unsigned i : indices(E->getComponents())) {
29762976
auto &component = E->getComponents()[i];
2977+
auto memberLocator = CS.getConstraintLocator(
2978+
locator, ConstraintLocator::PathElement::getKeyPathComponent(i));
2979+
auto resultLocator = CS.getConstraintLocator(
2980+
memberLocator, ConstraintLocator::KeyPathComponentResult);
2981+
29772982
switch (auto kind = component.getKind()) {
29782983
case KeyPathExpr::Component::Kind::Invalid:
29792984
break;
@@ -2982,7 +2987,7 @@ namespace {
29822987
// This should only appear in resolved ASTs, but we may need to
29832988
// re-type-check the constraints during failure diagnosis.
29842989
case KeyPathExpr::Component::Kind::Property: {
2985-
auto memberTy = CS.createTypeVariable(locator,
2990+
auto memberTy = CS.createTypeVariable(resultLocator,
29862991
TVO_CanBindToLValue |
29872992
TVO_CanBindToNoEscape);
29882993
auto lookupName = kind == KeyPathExpr::Component::Kind::UnresolvedProperty
@@ -2992,8 +2997,6 @@ namespace {
29922997
auto refKind = lookupName.isSimpleName()
29932998
? FunctionRefKind::Unapplied
29942999
: FunctionRefKind::Compound;
2995-
auto memberLocator = CS.getConstraintLocator(E,
2996-
ConstraintLocator::PathElement::getKeyPathComponent(i));
29973000
CS.addValueMemberConstraint(base, lookupName,
29983001
memberTy,
29993002
CurDC,
@@ -3008,11 +3011,8 @@ namespace {
30083011
// Subscript should only appear in resolved ASTs, but we may need to
30093012
// re-type-check the constraints during failure diagnosis.
30103013
case KeyPathExpr::Component::Kind::Subscript: {
3011-
3012-
auto *locator = CS.getConstraintLocator(E,
3013-
ConstraintLocator::PathElement::getKeyPathComponent(i));
30143014
base = addSubscriptConstraints(E, base, component.getIndexExpr(),
3015-
/*decl*/ nullptr, locator);
3015+
/*decl*/ nullptr, memberLocator);
30163016
break;
30173017
}
30183018

@@ -3026,18 +3026,20 @@ namespace {
30263026

30273027
// We can't assign an optional back through an optional chain
30283028
// today. Force the base to an rvalue.
3029-
auto rvalueTy = CS.createTypeVariable(locator, TVO_CanBindToNoEscape);
3030-
CS.addConstraint(ConstraintKind::Equal, base, rvalueTy, locator);
3029+
auto rvalueTy = CS.createTypeVariable(resultLocator,
3030+
TVO_CanBindToNoEscape);
3031+
CS.addConstraint(ConstraintKind::Equal, base, rvalueTy,
3032+
resultLocator);
30313033
base = rvalueTy;
30323034
LLVM_FALLTHROUGH;
30333035
}
30343036
case KeyPathExpr::Component::Kind::OptionalForce: {
3035-
auto optionalObjTy = CS.createTypeVariable(locator,
3037+
auto optionalObjTy = CS.createTypeVariable(resultLocator,
30363038
TVO_CanBindToLValue |
30373039
TVO_CanBindToNoEscape);
30383040

30393041
CS.addConstraint(ConstraintKind::OptionalObject, base, optionalObjTy,
3040-
locator);
3042+
resultLocator);
30413043

30423044
base = optionalObjTy;
30433045
break;
@@ -3052,6 +3054,10 @@ namespace {
30523054
case KeyPathExpr::Component::Kind::Identity:
30533055
continue;
30543056
}
3057+
3058+
// By now, `base` is the result type of this component. Set it in the
3059+
// constraint system so we can find it later.
3060+
CS.setType(E, i, base);
30553061
}
30563062

30573063
// If there was an optional chaining component, the end result must be
@@ -3078,10 +3084,8 @@ namespace {
30783084
} else {
30793085
// The type of key path depends on the overloads chosen for the key
30803086
// path components.
3081-
kpTy = CS.createTypeVariable(CS.getConstraintLocator(E),
3082-
TVO_CanBindToNoEscape);
3083-
CS.addKeyPathConstraint(kpTy, root, rvalueBase,
3084-
CS.getConstraintLocator(E));
3087+
kpTy = CS.createTypeVariable(locator, TVO_CanBindToNoEscape);
3088+
CS.addKeyPathConstraint(kpTy, root, rvalueBase, locator);
30853089
}
30863090
return kpTy;
30873091
}

lib/Sema/CSSolver.cpp

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,21 +1147,9 @@ ConstraintSystem::solveImpl(Expr *&expr,
11471147
}
11481148

11491149
if (TC.getLangOpts().DebugConstraintSolver) {
1150-
auto getTypeOfExpr = [&](const Expr *E) -> Type {
1151-
if (hasType(E))
1152-
return getType(E);
1153-
return Type();
1154-
};
1155-
auto getTypeOfTypeLoc = [&](const TypeLoc &TL) -> Type {
1156-
if (hasType(TL))
1157-
return getType(TL);
1158-
return Type();
1159-
};
1160-
11611150
auto &log = getASTContext().TypeCheckerDebug->getStream();
11621151
log << "---Initial constraints for the given expression---\n";
1163-
1164-
expr->dump(log, getTypeOfExpr, getTypeOfTypeLoc);
1152+
print(log, expr);
11651153
log << "\n";
11661154
print(log);
11671155
}

0 commit comments

Comments
 (0)