Skip to content

[3.1] Improve compile-time performance of string interpolation #7146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 11 additions & 39 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2099,45 +2099,32 @@ namespace {
// Create a tuple containing all of the segments.
SmallVector<Expr *, 4> segments;
SmallVector<Identifier, 4> names;
unsigned index = 0;
ConstraintLocatorBuilder locatorBuilder(cs.getConstraintLocator(expr));
for (auto segment : expr->getSegments()) {
auto locator = cs.getConstraintLocator(
locatorBuilder.withPathElement(
LocatorPathElt::getInterpolationArgument(index++)));

// Find the initializer we chose.
auto choice = getOverloadChoice(locator);

auto memberRef = buildMemberRef(
typeRef, choice.openedFullType,
segment->getStartLoc(), choice.choice.getDecl(),
DeclNameLoc(segment->getStartLoc()),
choice.openedType,
locator, locator, /*Implicit=*/true,
choice.choice.getFunctionRefKind(),
AccessSemantics::Ordinary,
/*isDynamic=*/false);
ApplyExpr *apply =
CallExpr::createImplicit(
tc.Context, memberRef,
tc.Context, typeRef,
{ segment },
{ tc.Context.Id_stringInterpolationSegment });
cs.cacheType(apply->getArg());

auto converted = finishApply(apply, openedType, locatorBuilder);
if (!converted)
return nullptr;
Expr *convertedSegment = apply;
if (tc.typeCheckExpressionShallow(convertedSegment, cs.DC))
continue;

segments.push_back(converted);
segments.push_back(convertedSegment);

if (index == 1) {
if (names.empty()) {
names.push_back(tc.Context.Id_stringInterpolation);
} else {
names.push_back(Identifier());
}
}

// If all of the segments had errors, bail out.
if (segments.empty())
return nullptr;

// Call the init(stringInterpolation:) initializer with the arguments.
ApplyExpr *apply = CallExpr::createImplicit(tc.Context, memberRef,
segments, names);
Expand Down Expand Up @@ -4115,22 +4102,7 @@ findCalleeDeclRef(ConstraintSystem &cs, const Solution &solution,

unsigned newFlags = locator->getSummaryFlags();

// If we have an interpolation argument, dig out the constructor if we
// can.
// FIXME: This representation is actually quite awful
if (newPath.size() == 1 &&
newPath[0].getKind() == ConstraintLocator::InterpolationArgument) {
newPath.push_back(ConstraintLocator::ConstructorMember);

locator = cs.getConstraintLocator(locator->getAnchor(), newPath, newFlags);
auto known = solution.overloadChoices.find(locator);
if (known != solution.overloadChoices.end()) {
auto &choice = known->second.choice;
if (choice.getKind() == OverloadChoiceKind::Decl)
return cast<AbstractFunctionDecl>(choice.getDecl());
}
return nullptr;
} else if (isSubscript) {
if (isSubscript) {
newPath.push_back(ConstraintLocator::SubscriptMember);
} else {
newPath.push_back(ConstraintLocator::ApplyFunction);
Expand Down
16 changes: 0 additions & 16 deletions lib/Sema/CSDiag.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,22 +222,6 @@ void constraints::simplifyLocator(Expr *&anchor,
}
break;

case ConstraintLocator::InterpolationArgument:
if (auto interp = dyn_cast<InterpolatedStringLiteralExpr>(anchor)) {
unsigned index = path[0].getValue();
if (index < interp->getSegments().size()) {
// No additional target locator information.
// FIXME: Dig out the constructor we're trying to call?
targetAnchor = nullptr;
targetPath.clear();

anchor = interp->getSegments()[index];
path = path.slice(1);
continue;
}
}
break;

case ConstraintLocator::SubscriptIndex:
if (auto subscript = dyn_cast<SubscriptExpr>(anchor)) {
targetAnchor = subscript->getBase();
Expand Down
53 changes: 28 additions & 25 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1022,6 +1022,16 @@ namespace {
DeclContext *CurDC;
SmallVector<DeclContext*, 4> DCStack;

static const unsigned numEditorPlaceholderVariables = 2;

/// A buffer of type variables used for editor placeholders. We only
/// use a small number of these (rotating through), to prevent expressions
/// with a large number of editor placeholders from flooding the constraint
/// system with type variables.
TypeVariableType *editorPlaceholderVariables[numEditorPlaceholderVariables]
= { nullptr, nullptr };
unsigned currentEditorPlaceholderVariable = 0;

/// \brief Add constraints for a reference to a named member of the given
/// base type, and return the type of such a reference.
Type addMemberRefConstraints(Expr *expr, Expr *base, DeclName name,
Expand Down Expand Up @@ -1224,7 +1234,6 @@ namespace {
visitInterpolatedStringLiteralExpr(InterpolatedStringLiteralExpr *expr) {
// Dig out the ExpressibleByStringInterpolation protocol.
auto &tc = CS.getTypeChecker();
auto &C = CS.getASTContext();
auto interpolationProto
= tc.getProtocol(expr->getLoc(),
KnownProtocolKind::ExpressibleByStringInterpolation);
Expand All @@ -1241,26 +1250,6 @@ namespace {
interpolationProto->getDeclaredType(),
locator);

// Each of the segments is passed as an argument to
// init(stringInterpolationSegment:).
unsigned index = 0;
auto tvMeta = MetatypeType::get(tv);
for (auto segment : expr->getSegments()) {
auto locator = CS.getConstraintLocator(
expr,
LocatorPathElt::getInterpolationArgument(index++));
auto segmentTyV = CS.createTypeVariable(locator, /*options=*/0);
auto returnTyV = CS.createTypeVariable(locator, /*options=*/0);
auto methodTy = FunctionType::get(segmentTyV, returnTyV);

CS.addConstraint(ConstraintKind::Conversion, CS.getType(segment),
segmentTyV, locator);

DeclName segmentName(C, C.Id_init, { C.Id_stringInterpolationSegment });
CS.addValueMemberConstraint(tvMeta, segmentName, methodTy, CurDC,
FunctionRefKind::DoubleApply, locator);
}

return tv;
}

Expand Down Expand Up @@ -2760,14 +2749,28 @@ namespace {
Type visitEditorPlaceholderExpr(EditorPlaceholderExpr *E) {
if (E->getTypeLoc().isNull()) {
auto locator = CS.getConstraintLocator(E);
auto placeholderTy = CS.createTypeVariable(locator, /*options*/0);

// A placeholder may have any type, but default to Void type if
// otherwise unconstrained.
CS.addConstraint(ConstraintKind::Defaultable,
placeholderTy, TupleType::getEmpty(CS.getASTContext()),
locator);
auto &placeholderTy
= editorPlaceholderVariables[currentEditorPlaceholderVariable];
if (!placeholderTy) {
placeholderTy = CS.createTypeVariable(locator, /*options*/0);

CS.addConstraint(ConstraintKind::Defaultable,
placeholderTy,
TupleType::getEmpty(CS.getASTContext()),
locator);
}

// Move to the next placeholder variable.
currentEditorPlaceholderVariable
= (currentEditorPlaceholderVariable + 1) %
numEditorPlaceholderVariables;

return placeholderTy;
}

// NOTE: The type loc may be there but have failed to validate, in which
// case we return the null type.
return E->getType();
Expand Down
11 changes: 0 additions & 11 deletions lib/Sema/CSSolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2465,17 +2465,6 @@ bool ConstraintSystem::solveSimplified(
if (*restriction == ConversionRestrictionKind::TupleToTuple)
break;
}

// Or, if we see a conversion successfully applied to a string
// interpolation argument, we're done.
// FIXME: Probably should be more general, as mentioned above.
if (auto locator = disjunction->getLocator()) {
if (!locator->getPath().empty() &&
locator->getPath().back().getKind()
== ConstraintLocator::InterpolationArgument &&
constraint->getKind() == ConstraintKind::Conversion)
break;
}
}

if (TC.getLangOpts().DebugConstraintSolver) {
Expand Down
5 changes: 0 additions & 5 deletions lib/Sema/ConstraintLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ void ConstraintLocator::Profile(llvm::FoldingSetNodeID &id, Expr *anchor,
case ScalarToTuple:
case Load:
case GenericArgument:
case InterpolationArgument:
case NamedTupleElement:
case TupleElement:
case ApplyArgToParam:
Expand Down Expand Up @@ -164,10 +163,6 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) {
out << "instance type";
break;

case InterpolationArgument:
out << "interpolation argument #" << llvm::utostr(elt.getValue());
break;

case Load:
out << "load";
break;
Expand Down
10 changes: 0 additions & 10 deletions lib/Sema/ConstraintLocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ class ConstraintLocator : public llvm::FoldingSetNode {
SubscriptIndex,
/// \brief The result of a subscript expression.
SubscriptResult,
/// \brief An argument to string interpolation.
InterpolationArgument,
/// \brief The lookup for a constructor member.
ConstructorMember,
/// \brief Rvalue adjustment.
Expand Down Expand Up @@ -155,7 +153,6 @@ class ConstraintLocator : public llvm::FoldingSetNode {
return 0;

case GenericArgument:
case InterpolationArgument:
case NamedTupleElement:
case TupleElement:
return 1;
Expand Down Expand Up @@ -204,7 +201,6 @@ class ConstraintLocator : public llvm::FoldingSetNode {
case Archetype:
case AssociatedType:
case GenericArgument:
case InterpolationArgument:
case NamedTupleElement:
case TupleElement:
case Requirement:
Expand Down Expand Up @@ -335,12 +331,6 @@ class ConstraintLocator : public llvm::FoldingSetNode {
return PathElement(GenericArgument, position);
}

/// \brief Retrieve a path element for an argument to string
/// interpolation.
static PathElement getInterpolationArgument(unsigned position) {
return PathElement(InterpolationArgument, position);
}

/// \brief Retrieve the kind of path element.
PathElementKind getKind() const {
switch (static_cast<StoredKind>(storedKind)) {
Expand Down
18 changes: 12 additions & 6 deletions stdlib/public/core/StringInterpolation.swift.gyb
Original file line number Diff line number Diff line change
Expand Up @@ -68,29 +68,35 @@ extension String : _ExpressibleByStringInterpolation {
self = String(describing: expr)
}

% for Type in StreamableTypes:
/// Creates a string containing the given value's textual representation.
///
/// Do not call this initializer directly. It is used by the compiler when
/// interpreting string interpolations.
///
/// - SeeAlso: `ExpressibleByStringInterpolation`
public init(stringInterpolationSegment expr: ${Type}) {
public init<T: TextOutputStreamable> (stringInterpolationSegment expr: T) {
self = _toStringReadOnlyStreamable(expr)
}
% end

% for Type in PrintableTypes:
/// Creates a string containing the given value's textual representation.
///
/// Do not call this initializer directly. It is used by the compiler when
/// interpreting string interpolations.
///
/// - SeeAlso: `ExpressibleByStringInterpolation`
public init(stringInterpolationSegment expr: ${Type}) {
public init<T: CustomStringConvertible> (stringInterpolationSegment expr: T) {
self = _toStringReadOnlyPrintable(expr)
}
% end

/// Creates a string containing the given value's textual representation.
///
/// Do not call this initializer directly. It is used by the compiler when
/// interpreting string interpolations.
///
/// - SeeAlso: `ExpressibleByStringInterpolation`
public init<T: TextOutputStreamable & CustomStringConvertible> (stringInterpolationSegment expr: T) {
self = _toStringReadOnlyStreamable(expr)
}
}

// ${'Local Variables'}:
Expand Down
45 changes: 45 additions & 0 deletions test/Constraints/interpolation_segments.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// RUN: %target-typecheck-verify-swift -typecheck -debug-constraints %s > %t.dump 2>&1
// RUN: %FileCheck %s < %t.dump

// Make sure that the interpolation segments get placed into separate connected
// components.
// CHECK: ---Connected components---
// CHECK-NEXT: 0:
// CHECK-NEXT: 1:
// CHECK-NEXT: 2:
// CHECK-NEXT: 3:
// CHECK-NEXT: 4:
// CHECK-NEXT: 5:
// CHECK-NEXT: 6:
// CHECK-NEXT: 7:
// CHECK-NEXT: 8:
// CHECK-NEXT: 9:

// CHECK: (solving component #
// CHECK: literal=3 bindings=(subtypes of) (default from ExpressibleByStringLiteral) String)

// CHECK: (solving component #
// CHECK: literal=3 bindings=(subtypes of) (default from ExpressibleByIntegerLiteral) Int)

// CHECK: (solving component #
// CHECK: literal=3 bindings=(subtypes of) (default from ExpressibleByStringLiteral) String)

// CHECK: (solving component #
// CHECK: literal=3 bindings=(subtypes of) (default from ExpressibleByIntegerLiteral) Int)

// CHECK: (solving component #
// CHECK: literal=3 bindings=(subtypes of) (default from ExpressibleByStringLiteral) String)

// CHECK: (solving component #
// CHECK: literal=3 bindings=(subtypes of) (default from ExpressibleByIntegerLiteral) Int)

// CHECK: (solving component #
// CHECK: literal=3 bindings=(subtypes of) (default from ExpressibleByStringLiteral) String)

// CHECK: (solving component #
// CHECK: literal=3 bindings=(subtypes of) (default from ExpressibleByIntegerLiteral) Int)

// CHECK: (solving component #
// CHECK: literal=3 bindings=(subtypes of) (default from ExpressibleByStringLiteral) String)

_ = "\(1), \(2), \(3), \(4)"
2 changes: 2 additions & 0 deletions test/api-digester/source-stability.swift.expected
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Protocol IndexableBase has been removed (deprecated)
Protocol MutableIndexable has been removed (deprecated)
Protocol RandomAccessIndexable has been removed (deprecated)
Protocol RangeReplaceableIndexable has been removed (deprecated)
Constructor String.init(stringInterpolationSegment:) has been removed
Func Array.append(contentsOf:) has been removed
Func ArraySlice.append(contentsOf:) has been removed
Func ContiguousArray.append(contentsOf:) has been removed
Expand All @@ -34,6 +35,7 @@ Constructor RangeReplaceableBidirectionalSlice.init(base:bounds:) has 2nd parame
Constructor RangeReplaceableRandomAccessSlice.init(base:bounds:) has 2nd parameter type change from Range<Base.Index> to Range<RangeReplaceableRandomAccessSlice.Index>
Constructor RangeReplaceableSlice.init(base:bounds:) has 2nd parameter type change from Range<Base.Index> to Range<RangeReplaceableSlice.Index>
Constructor Slice.init(base:bounds:) has 2nd parameter type change from Range<Base.Index> to Range<Slice.Index>
Constructor String.init(stringInterpolationSegment:) has 1st parameter type change from String to T
Func AnyBidirectionalCollection.makeIterator() has return type change from AnyIterator<Element> to AnyBidirectionalCollection.Iterator
Func AnyCollection.makeIterator() has return type change from AnyIterator<Element> to AnyCollection.Iterator
Func AnyRandomAccessCollection.makeIterator() has return type change from AnyIterator<Element> to AnyRandomAccessCollection.Iterator
Expand Down
4 changes: 4 additions & 0 deletions validation-test/Sema/interpolation_placeholders.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// RUN: not %target-swift-frontend -typecheck %s

let query = "<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)&<#...#>=\(<#...#>)"