Skip to content

[5.9][ConstraintSystem] Some more variadic generic fixes #66173

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
merged 5 commits into from
May 29, 2023
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
4 changes: 3 additions & 1 deletion include/swift/AST/PackConformance.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ class alignas(1 << DeclAlignInBits) PackConformance final

ArrayRef<ProtocolConformanceRef> getPatternConformances() const;

bool isInvalid() const;

bool isCanonical() const;

PackConformance *getCanonicalConformance() const;
Expand Down Expand Up @@ -103,4 +105,4 @@ void simple_display(llvm::raw_ostream &out, PackConformance *conformance);

} // end namespace swift

#endif // SWIFT_AST_PACKCONFORMANCE_H
#endif // SWIFT_AST_PACKCONFORMANCE_H
23 changes: 23 additions & 0 deletions include/swift/AST/PackExpansionMatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,29 @@ class TypeListPackMatcher {
}
}

// If both sides have the same number of elements and all of
// them are pack expansions there is not going to be any
// expansion "absorption" and it's okay to match per-index.
//
// Like in all previous cases the callers are responsible
// to check whether the element types actually line up,
// this is a purely structural match.
if (lhsElts.size() == rhsElts.size()) {
for (unsigned i = 0, n = lhsElts.size(); i != n; ++i) {
auto lhsType = getElementType(lhsElts[i]);
auto rhsType = getElementType(rhsElts[i]);

if (IsPackExpansionType(lhsType) && IsPackExpansionType(rhsType)) {
pairs.emplace_back(lhsType, rhsType, i, i);
} else {
pairs.clear();
return true;
}
}

return false;
}

// Otherwise, all remaining possibilities are invalid:
// - Neither side has any pack expansions, and they have different lengths.
// - One side has a pack expansion but the other side is too short, eg
Expand Down
7 changes: 5 additions & 2 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1827,8 +1827,11 @@ static ProtocolConformanceRef getPackTypeConformance(
patternConformances.push_back(patternConformance);
}

return ProtocolConformanceRef(
PackConformance::get(type, protocol, patternConformances));
auto *conformance = PackConformance::get(type, protocol, patternConformances);
if (conformance->isInvalid())
return ProtocolConformanceRef::forInvalid();

return ProtocolConformanceRef(conformance);
}

ProtocolConformanceRef
Expand Down
8 changes: 8 additions & 0 deletions lib/AST/PackConformance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ size_t PackConformance::numTrailingObjects(
return ConformingType->getNumElements();
}

bool PackConformance::isInvalid() const {
return llvm::any_of(getPatternConformances(),
[&](const auto ref) { return ref.isInvalid(); });
}

ArrayRef<ProtocolConformanceRef>
PackConformance::getPatternConformances() const {
return {getTrailingObjects<ProtocolConformanceRef>(),
Expand Down Expand Up @@ -244,6 +249,9 @@ PackConformance::subst(InFlightSubstitution &IFS) const {

auto substConformance = PackConformance::get(substConformingType, Protocol,
expander.substConformances);
if (substConformance->isInvalid())
return ProtocolConformanceRef::forInvalid();

return ProtocolConformanceRef(substConformance);
}

Expand Down
9 changes: 7 additions & 2 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4547,8 +4547,13 @@ operator()(CanType dependentType, Type conformingReplacementType,
(*this)(dependentType, conformingPackElt, conformedProtocol);
conformances.push_back(conformance);
}
return ProtocolConformanceRef(
PackConformance::get(conformingPack, conformedProtocol, conformances));

auto *conformance =
PackConformance::get(conformingPack, conformedProtocol, conformances);
if (conformance->isInvalid())
return ProtocolConformanceRef::forInvalid();

return ProtocolConformanceRef(conformance);
}

assert((conformingReplacementType->is<ErrorType>() ||
Expand Down
7 changes: 7 additions & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8393,6 +8393,13 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
if (path.back().is<LocatorPathElt::PackElement>())
path.pop_back();

// This is similar to `PackElement` but locator points to the requirement
// associted with pack expansion pattern (i.e. `repeat each T: P`) where
// the path is something like:
// `... -> type req # -> pack expansion pattern`.
if (path.back().is<LocatorPathElt::PackExpansionPattern>())
path.pop_back();

if (auto req = path.back().getAs<LocatorPathElt::AnyRequirement>()) {
// If this is a requirement associated with `Self` which is bound
// to `Any`, let's consider this "too incorrect" to continue.
Expand Down
13 changes: 10 additions & 3 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6290,11 +6290,18 @@ Solution::getFunctionArgApplyInfo(ConstraintLocator *locator) const {
*choice, [this](Type type) -> Type { return simplifyType(type); }))
fnInterfaceType = fnInterfaceType->castTo<AnyFunctionType>()->getResult();

#ifndef NDEBUG
// If variadic generics are not involved, interface type should
// always match applied type.
if (auto *fn = fnInterfaceType->getAs<AnyFunctionType>()) {
assert(fn->getNumParams() == fnType->getNumParams() &&
"Parameter mismatch?");
(void)fn;
if (llvm::none_of(fn->getParams(), [&](const auto &param) {
return param.getPlainType()->hasParameterPack();
})) {
assert(fn->getNumParams() == fnType->getNumParams() &&
"Parameter mismatch?");
}
}
#endif
} else {
fnInterfaceType = resolveInterfaceType(rawFnType);
}
Expand Down
11 changes: 11 additions & 0 deletions test/Constraints/pack-expansion-expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -529,3 +529,14 @@ func test_pack_expansion_to_void_conv_for_closure_result<each T>(x: repeat each
let _: (Int) -> Void = { repeat ($0, print(each x)) } // expected-warning {{'repeat (Int, ())' is unused}}
let _: (Int, String) -> Void = { ($0, repeat ($1, print(each x))) } // expected-warning {{'(Int, repeat (String, ()))' is unused}}
}

// rdar://109539394 - crash on passing multiple variadic lists to singly variadic callee
do {
func test1<each T>(_: repeat each T) {}
func test2<each T>(_: repeat each T) where repeat each T: RawRepresentable {} // expected-note {{where 'each T' = 'each T2'}}

func caller<each T1, each T2>(t1: repeat each T1, t2: repeat each T2) {
test1(repeat each t1, repeat each t2) // Ok
test2(repeat each t2, repeat each t1) // expected-error {{local function 'test2' requires that 'each T2' conform to 'RawRepresentable'}}
}
}