From af145375fd74ae4134a437c836aa10d06bec48d8 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Thu, 4 May 2023 13:00:08 -0700 Subject: [PATCH] Fix a performance issue when answering "is this tuple Copyable"? If we don't split up the tuple into individual constraints, we end up spending more time querying whether a tuple is Copyable in `lookupConformance`, because it will naively check the types of all elements of the tuple recursively with `lookupConformance`. This is inefficient because if we know some of the elements of the tuple are fixed types, we don't need to keep checking those again. For example, if we have `($T, Int, $U)`, and then try a binding for `$T`, we might ask again if the whole tuple conforms. Leading to `lookupConformance` to check whether `Int` (and all other elements of the tuple) conforms to Copyable, when we either already know that, or can't answer it yet because it's still a type variable. By splitting up a Copyable constraint on a tuple into invidivual constraints on each of its type elements, we can avoid this redundant work by `lookupConformance`. While today we could short-cut this even further to say that _all_ tuples are Copyable, since we emit an error if a noncopyable type appears in one, that won't be true in the near future. This is the nicer solution we'll want to keep around long-term. After discussing this with Pavel, we don't think there's a good way to add a regression test for this, because the performance issue primarily comes up in specific example programs that aren't amenable to scale tests. resolves rdar://107536402 (cherry picked from commit ce04b84489fdd7f8ecaacbbd3bb8363234466f23) --- lib/Sema/CSSimplify.cpp | 15 +++++++++++++++ test/Constraints/moveonly_constraints.swift | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 3f20ec0483219..c2249e514e57f 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -8200,6 +8200,21 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint( return SolutionKind::Solved; } + // Copyable is checked structurally, so for better performance, split apart + // this constraint into individual Copyable constraints on each tuple element. + if (auto *tupleType = type->getAs()) { + if (protocol->isSpecificProtocol(KnownProtocolKind::Copyable)) { + for (unsigned i = 0, e = tupleType->getNumElements(); i < e; ++i) { + addConstraint(ConstraintKind::ConformsTo, + tupleType->getElementType(i), + protocol->getDeclaredInterfaceType(), + locator.withPathElement(LocatorPathElt::TupleElement(i))); + } + + return SolutionKind::Solved; + } + } + auto *loc = getConstraintLocator(locator); /// Record the given conformance as the result, adding any conditional diff --git a/test/Constraints/moveonly_constraints.swift b/test/Constraints/moveonly_constraints.swift index 188efa795c19d..772b0df439e86 100644 --- a/test/Constraints/moveonly_constraints.swift +++ b/test/Constraints/moveonly_constraints.swift @@ -88,8 +88,8 @@ func testBasic(_ mo: borrowing MO) { genericVarArg(5) genericVarArg(mo) // expected-error {{move-only type 'MO' cannot be used with generics yet}} - takeGeneric( (mo, 5) ) // expected-error {{global function 'takeGeneric' requires that 'MO' conform to '_Copyable'}} - takeGenericSendable((mo, mo)) // expected-error 2{{global function 'takeGenericSendable' requires that 'MO' conform to '_Copyable'}} + takeGeneric( (mo, 5) ) // expected-error {{move-only type 'MO' cannot be used with generics yet}} + takeGenericSendable((mo, mo)) // expected-error 2{{move-only type 'MO' cannot be used with generics yet}} let singleton : (MO) = (mo) takeGeneric(singleton) // expected-error {{move-only type 'MO' cannot be used with generics yet}}