Skip to content

[Variadic Generics] drop requirement of .element for tuple expansion #66214

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
6 changes: 3 additions & 3 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -3679,15 +3679,15 @@ class PackExpansionExpr final : public Expr {
};

/// An expression to materialize a pack from a tuple containing a pack
/// expansion, spelled \c tuple.element.
/// expansion.
///
/// These nodes are created by CSApply and should only appear in a
/// type-checked AST in the context of a \c PackExpansionExpr .
class MaterializePackExpr final : public Expr {
/// The expression from which to materialize a pack.
Expr *FromExpr;

/// The source location of \c .element
/// The source location of (deprecated) \c .element
SourceLoc ElementLoc;

MaterializePackExpr(Expr *fromExpr, SourceLoc elementLoc,
Expand All @@ -3711,7 +3711,7 @@ class MaterializePackExpr final : public Expr {
}

SourceLoc getEndLoc() const {
return ElementLoc;
return ElementLoc.isInvalid() ? ElementLoc : FromExpr->getEndLoc();
}

static bool classof(const Expr *E) {
Expand Down
1 change: 1 addition & 0 deletions include/swift/AST/KnownIdentifiers.def
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,7 @@ IDENTIFIER(size)
IDENTIFIER(speed)
IDENTIFIER(unchecked)
IDENTIFIER(unsafe)
IDENTIFIER(element)

// The singleton instance of TupleTypeDecl in the Builtin module
IDENTIFIER(TheTupleType)
Expand Down
2 changes: 2 additions & 0 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -6100,6 +6100,8 @@ Type isPlaceholderVar(PatternBindingDecl *PB);
/// Dump an anchor node for a constraint locator or contextual type.
void dumpAnchor(ASTNode anchor, SourceManager *SM, raw_ostream &out);

bool isSingleUnlabeledPackExpansionTuple(Type type);

} // end namespace constraints

template<typename ...Args>
Expand Down
3 changes: 1 addition & 2 deletions include/swift/Sema/OverloadChoice.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ enum class OverloadChoiceKind : int {
/// The overload choice selects a particular declaration that
/// was found by unwrapping an optional context type.
DeclViaUnwrappedOptional,
/// The overload choice materializes a pack from a tuple using
/// the \c .element syntax.
/// The overload choice materializes a pack from a tuple.
MaterializePack,
/// The overload choice indexes into a tuple. Index zero will
/// have the value of this enumerator, index one will have the value of this
Expand Down
1 change: 0 additions & 1 deletion lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5223,7 +5223,6 @@ void PrintAST::visitPackExpansionExpr(PackExpansionExpr *expr) {

void PrintAST::visitMaterializePackExpr(MaterializePackExpr *expr) {
visit(expr->getFromExpr());
Printer << ".element";
}

void PrintAST::visitPackElementExpr(PackElementExpr *expr) {
Expand Down
15 changes: 15 additions & 0 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3846,6 +3846,21 @@ namespace {
}

Expr *visitPackElementExpr(PackElementExpr *expr) {
if (auto *packRefExpr = expr->getPackRefExpr()) {
packRefExpr = cs.coerceToRValue(packRefExpr);
auto packRefType = cs.getType(packRefExpr);
if (auto *tuple = packRefType->getRValueType()->getAs<TupleType>();
tuple && tuple->isSingleUnlabeledPackExpansion()) {
auto *expansion =
tuple->getElementType(0)->castTo<PackExpansionType>();
auto patternType = expansion->getPatternType();
auto *materializedPackExpr = MaterializePackExpr::create(
cs.getASTContext(), packRefExpr, packRefExpr->getLoc(),
patternType, /*implicit*/ true);
cs.cacheType(materializedPackExpr);
expr->setPackRefExpr(materializedPackExpr);
}
}
return simplifyExprType(expr);
}

Expand Down
22 changes: 16 additions & 6 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3148,10 +3148,11 @@ namespace {
if (type->is<ElementArchetypeType>() &&
CS.hasFixFor(CS.getConstraintLocator(declRef),
FixKind::IgnoreMissingEachKeyword)) {
Packs.push_back(PackElementExpr::create(CS.getASTContext(),
/*eachLoc=*/SourceLoc(),
declRef,
/*implicit=*/true));
auto *packElementExpr =
PackElementExpr::create(CS.getASTContext(),
/*eachLoc=*/{}, declRef,
/*implicit=*/true, type);
Packs.push_back(CS.cacheType(packElementExpr));
}
}

Expand Down Expand Up @@ -3230,8 +3231,17 @@ namespace {
}

Type visitPackElementExpr(PackElementExpr *expr) {
return openPackElement(CS.getType(expr->getPackRefExpr()),
CS.getConstraintLocator(expr));
auto packType = CS.getType(expr->getPackRefExpr());

if (isSingleUnlabeledPackExpansionTuple(packType)) {
packType =
addMemberRefConstraints(expr, expr->getPackRefExpr(),
DeclNameRef(CS.getASTContext().Id_element),
FunctionRefKind::Unapplied, {});
CS.setType(expr->getPackRefExpr(), packType);
}

return openPackElement(packType, CS.getConstraintLocator(expr));
}

Type visitMaterializePackExpr(MaterializePackExpr *expr) {
Expand Down
25 changes: 25 additions & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,14 @@ static bool isPackExpansionType(Type type) {
return false;
}

bool constraints::isSingleUnlabeledPackExpansionTuple(Type type) {
auto *tuple = type->getRValueType()->getAs<TupleType>();
// TODO: drop no name requirement
return tuple && (tuple->getNumElements() == 1) &&
isPackExpansionType(tuple->getElementType(0)) &&
!tuple->getElement(0).hasName();
}

static bool containsPackExpansionType(ArrayRef<AnyFunctionType::Param> params) {
return llvm::any_of(params, [&](const auto &param) {
return isPackExpansionType(param.getPlainType());
Expand Down Expand Up @@ -9121,6 +9129,16 @@ ConstraintSystem::simplifyPackElementOfConstraint(Type first, Type second,
return SolutionKind::Solved;
}

if (isSingleUnlabeledPackExpansionTuple(patternType)) {
auto *elementVar =
createTypeVariable(getConstraintLocator(locator), /*options=*/0);
addValueMemberConstraint(
patternType, DeclNameRef(getASTContext().Id_element), elementVar, DC,
FunctionRefKind::Unapplied, {},
getConstraintLocator(locator, {ConstraintLocator::Member}));
patternType = elementVar;
}

// Let's try to resolve element type based on the pattern type.
if (!patternType->hasTypeVariable()) {
auto *loc = getConstraintLocator(locator);
Expand Down Expand Up @@ -9338,6 +9356,7 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
if (!memberName.isSpecial()) {
StringRef nameStr = memberName.getBaseIdentifier().str();
// Accessing `.element` on an abstract tuple materializes a pack.
// (deprecated behavior)
if (nameStr == "element" && baseTuple->getNumElements() == 1 &&
isPackExpansionType(baseTuple->getElementType(0))) {
auto elementType = baseTuple->getElementType(0);
Expand Down Expand Up @@ -13312,6 +13331,12 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyShapeOfConstraint(
return SolutionKind::Solved;
}

// Map element archetypes to the pack context to check for equality.
if (packTy->hasElementArchetype()) {
auto *packEnv = DC->getGenericEnvironmentOfContext();
packTy = packEnv->mapElementTypeIntoPackContext(packTy);
}

auto shape = packTy->getReducedShape();
addConstraint(ConstraintKind::Bind, shapeTy, shape, locator);
return SolutionKind::Solved;
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3559,7 +3559,7 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
break;

case OverloadChoiceKind::MaterializePack: {
// Since `.element` is only applicable to single element tuples at the
// Since pack expansion is only applicable to single element tuples at the
// moment we can just look through l-value base to load it.
//
// In the future, _if_ the syntax allows for multiple expansions
Expand Down
34 changes: 24 additions & 10 deletions test/Constraints/pack-expansion-expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,18 @@ func expansionOfNonPackType<T>(_ t: repeat each T) {}

func tupleExpansion<each T, each U>(
_ tuple1: (repeat each T),
_ tuple2: (repeat each U)
_ tuple2: (repeat each U),
_ tuple3: inout (repeat each T)
) {
_ = forward(repeat each tuple1.element)
_ = forward(repeat each tuple1)

_ = zip(repeat each tuple1.element, with: repeat each tuple1.element)
_ = zip(repeat each tuple1, with: repeat each tuple1)
_ = zip(repeat each tuple1, with: repeat each tuple1.element) // legacy syntax

_ = zip(repeat each tuple1.element, with: repeat each tuple2.element)
_ = zip(repeat each tuple1, with: repeat each tuple2)
// expected-error@-1 {{global function 'zip(_:with:)' requires the type packs 'each T' and 'each U' have the same shape}}

_ = forward(repeat each tuple3)
}

protocol Generatable {
Expand Down Expand Up @@ -243,10 +247,10 @@ func test_pack_expansion_materialization_from_lvalue_base() {

init() {
self.data = (repeat Data<each T>())
_ = (repeat each data.element) // Ok
_ = (repeat each data) // Ok

var tmp = (repeat Data<each T>()) // expected-warning {{never mutated}}
_ = (repeat each tmp.element) // Ok
_ = (repeat each tmp) // Ok

// TODO: Add subscript test-case when syntax is supported.
}
Expand Down Expand Up @@ -274,10 +278,9 @@ func packOutsideExpansion<each T>(_ t: repeat each T) {

let tuple = (repeat each t)

_ = tuple.element
// expected-error@-1{{pack reference 'each T' can only appear in pack expansion}}
_ = tuple

_ = each tuple.element
_ = each tuple
// expected-error@-1{{pack reference 'each T' can only appear in pack expansion}}
}

Expand Down Expand Up @@ -481,7 +484,6 @@ do {
func test_misplaced_each<each T: P>(_ value: repeat each T) -> (repeat each T.A) {
return (repeat each value.makeA())
// expected-error@-1 {{value pack 'each T' must be referenced with 'each'}} {{25-25=(each }} {{30-30=)}}
// expected-error@-2 {{pack expansion requires that '()' and 'each T' have the same shape}}
}
}

Expand Down Expand Up @@ -539,3 +541,15 @@ do {
test2(repeat each t2, repeat each t1) // expected-error {{local function 'test2' requires that 'each T2' conform to 'RawRepresentable'}}
}
}

do {
func overloaded<each T>(_ a: Int, _ b: repeat each T) -> (repeat each T) {
return (repeat each b)
}

func overloaded() {}

func test<each T>(_ a: repeat each T) {
_ = (repeat overloaded(1, each a))
}
}
4 changes: 2 additions & 2 deletions test/Interpreter/Inputs/variadic_generic_library.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ public struct Predicate<each Input> {
builder: (repeat Variable<each Input>) -> Expr
) where Expr: Expression<Bool> {
self.variables = (repeat Variable<each Input>())
self.expression = builder(repeat each variables.element)
self.expression = builder(repeat each variables)
}

public func evaluate(
_ input: repeat each Input
) throws -> Bool {
return try expression.evaluate(
.init(repeat (each variables.element, each input))
.init(repeat (each variables, each input))
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/Interpreter/variadic_generic_tuples.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func makeTuple<each Element>(

func expandTupleElements<each T: Equatable>(_ value: repeat each T) {
let values = makeTuple(repeat each value)
_ = (repeat expectEqual(each value, each values.element))
_ = (repeat expectEqual(each value, each values))
}

tuples.test("expandTuple") {
Expand Down
6 changes: 3 additions & 3 deletions test/SILGen/variadic-generic-tuples.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func wrapTupleElements<each T>(_ value: repeat each T) -> (repeat Wrapper<each T
// CHECK: [[VAR:%.*]] = alloc_stack [lexical] $(repeat each T)
let values = (repeat each value)

// Create a temporary for the 'values' in 'each values.element'
// Create a temporary for the 'values' in 'each values'
// CHECK: bb3:
// CHECK-NEXT: [[TEMP:%.*]] = alloc_stack $(repeat each T)
// CHECK-NEXT: copy_addr [[VAR]] to [init] [[TEMP]] : $*(repeat each T)
Expand Down Expand Up @@ -155,7 +155,7 @@ func wrapTupleElements<each T>(_ value: repeat each T) -> (repeat Wrapper<each T
// CHECK-NEXT: [[NEXT_INDEX:%.*]] = builtin "add_Word"([[INDEX]] : $Builtin.Word, [[ONE]] : $Builtin.Word) : $Builtin.Word
// CHECK-NEXT: br bb4([[NEXT_INDEX]] : $Builtin.Word)

return (repeat Wrapper(value: each values.element))
return (repeat Wrapper(value: each values))

// CHECK: destroy_addr [[TEMP]] : $*(repeat each T)
// CHECK: dealloc_stack [[TEMP]] : $*(repeat each T)
Expand Down Expand Up @@ -303,7 +303,7 @@ struct FancyTuple<each T> {
var x: (repeat each T)

func makeTuple() -> (repeat each T) {
return (repeat each x.element)
return (repeat each x)
}
}

Expand Down