Skip to content

[Variadic Generics] Always use PackType as the substitution for type parameter packs. #64166

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 6 commits into from
Mar 7, 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
3 changes: 3 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -2733,6 +2733,9 @@ ERROR(requires_not_suitable_archetype,none,
ERROR(invalid_shape_requirement,none,
"invalid same-shape requirement between %0 and %1",
(Type, Type))
ERROR(unsupported_same_element,none,
"same-element requirements are not yet supported",
())

ERROR(requires_generic_params_made_equal,none,
"same-type requirement makes generic parameters %0 and %1 equivalent",
Expand Down
9 changes: 4 additions & 5 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -1505,7 +1505,8 @@ class Solution {

/// The pack expansion environment that can open pack elements for
/// a given locator.
llvm::DenseMap<ConstraintLocator *, UUID> PackExpansionEnvironments;
llvm::DenseMap<ConstraintLocator *, std::pair<UUID, Type>>
PackExpansionEnvironments;

/// The locators of \c Defaultable constraints whose defaults were used.
llvm::SmallPtrSet<ConstraintLocator *, 2> DefaultedConstraints;
Expand Down Expand Up @@ -3067,7 +3068,8 @@ class ConstraintSystem {
llvm::SmallMapVector<ConstraintLocator *, OpenedArchetypeType *, 4>
OpenedExistentialTypes;

llvm::SmallMapVector<ConstraintLocator *, UUID, 4> PackExpansionEnvironments;
llvm::SmallMapVector<ConstraintLocator *, std::pair<UUID, Type>, 4>
PackExpansionEnvironments;

/// The set of functions that have been transformed by a result builder.
llvm::MapVector<AnyFunctionRef, AppliedBuilderTransform>
Expand Down Expand Up @@ -4031,9 +4033,6 @@ class ConstraintSystem {
std::pair<Type, OpenedArchetypeType *> openExistentialType(
Type type, ConstraintLocator *locator);

/// Add the given pack expansion as an opened pack element environment.
void addPackElementEnvironment(PackExpansionExpr *expr);

/// Get the opened element generic environment for the given locator.
GenericEnvironment *getPackElementEnvironment(ConstraintLocator *locator,
CanType shapeClass);
Expand Down
1 change: 1 addition & 0 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5147,6 +5147,7 @@ GenericEnvironment::forOpenedElement(GenericSignature signature,
if (found != openedElementEnvironments.end()) {
auto *existingEnv = found->second;
assert(existingEnv->getGenericSignature().getPointer() == signature.getPointer());
assert(existingEnv->getOpenedElementShapeClass()->isEqual(shapeClass));
assert(existingEnv->getOpenedElementUUID() == uuid);

return existingEnv;
Expand Down
8 changes: 6 additions & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4670,8 +4670,12 @@ static Type computeNominalType(NominalTypeDecl *decl, DeclTypeKind kind) {
// the generic parameter list directly instead of looking
// at the signature.
SmallVector<Type, 4> args;
for (auto param : decl->getGenericParams()->getParams())
args.push_back(param->getDeclaredInterfaceType());
for (auto param : decl->getGenericParams()->getParams()) {
auto argTy = param->getDeclaredInterfaceType();
if (param->isParameterPack())
argTy = PackType::getSingletonPackExpansion(argTy);
args.push_back(argTy);
}

return BoundGenericType::get(decl, ParentTy, args);
}
Expand Down
29 changes: 28 additions & 1 deletion lib/AST/GenericEnvironment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -681,10 +681,37 @@ GenericEnvironment::mapElementTypeIntoPackContext(Type type) const {
}, LookUpConformanceInSignature(sig.getPointer()));
}

namespace {
/// A function suitable for use as a \c TypeSubstitutionFn that produces
/// correct forwarding substitutions for a generic environment.
///
/// This differs from QueryInterfaceTypeSubstitutions only in that it
/// always produces PackTypes for pack parameters.
class BuildForwardingSubstitutions {
QueryInterfaceTypeSubstitutions Query;

public:
BuildForwardingSubstitutions(const GenericEnvironment *self)
: Query(self) { }

Type operator()(SubstitutableType *type) const;
};
} // end anonymous namespace

Type BuildForwardingSubstitutions::operator()(SubstitutableType *type) const {
if (auto resultType = Query(type)) {
auto param = type->castTo<GenericTypeParamType>();
if (!param->isParameterPack())
return resultType;
return PackType::getSingletonPackExpansion(resultType);
}
return Type();
}

SubstitutionMap GenericEnvironment::getForwardingSubstitutionMap() const {
auto genericSig = getGenericSignature();
return SubstitutionMap::get(genericSig,
QueryInterfaceTypeSubstitutions(this),
BuildForwardingSubstitutions(this),
MakeAbstractConformanceForGenericType());
}

Expand Down
11 changes: 7 additions & 4 deletions lib/AST/GenericSignature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -579,10 +579,13 @@ unsigned GenericParamKey::findIndexIn(

SubstitutionMap GenericSignatureImpl::getIdentitySubstitutionMap() const {
return SubstitutionMap::get(const_cast<GenericSignatureImpl *>(this),
[](SubstitutableType *t) -> Type {
return Type(cast<GenericTypeParamType>(t));
},
MakeAbstractConformanceForGenericType());
[](SubstitutableType *t) -> Type {
auto param = cast<GenericTypeParamType>(t);
if (!param->isParameterPack())
return param;
return PackType::getSingletonPackExpansion(param);
},
MakeAbstractConformanceForGenericType());
}

GenericTypeParamType *GenericSignatureImpl::getSugaredType(
Expand Down
9 changes: 9 additions & 0 deletions lib/AST/RequirementMachine/Diagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,15 @@ bool swift::rewriting::diagnoseRequirementErrors(

break;
}

case RequirementError::Kind::UnsupportedSameElement: {
if (error.requirement.hasError())
break;

ctx.Diags.diagnose(loc, diag::unsupported_same_element);
diagnosedError = true;
break;
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions lib/AST/RequirementMachine/Diagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ struct RequirementError {
RecursiveRequirement,
/// A redundant requirement, e.g. T == T.
RedundantRequirement,
/// A not-yet-supported same-element requirement, e.g. each T == Int.
UnsupportedSameElement,
} kind;

/// The invalid requirement.
Expand Down Expand Up @@ -100,6 +102,10 @@ struct RequirementError {
SourceLoc loc) {
return {Kind::RecursiveRequirement, req, loc};
}

static RequirementError forSameElement(Requirement req, SourceLoc loc) {
return {Kind::UnsupportedSameElement, req, loc};
}
};

/// Policy for the fixit that transforms 'T : S' where 'S' is not a protocol
Expand Down
9 changes: 9 additions & 0 deletions lib/AST/RequirementMachine/RequirementLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,15 @@ static void desugarSameTypeRequirement(Type lhs, Type rhs, SourceLoc loc,

bool mismatch(TypeBase *firstType, TypeBase *secondType,
Type sugaredFirstType) {
// If one side is a parameter pack, this is a same-element requirement, which
// is not yet supported.
if (firstType->isParameterPack() != secondType->isParameterPack()) {
errors.push_back(RequirementError::forSameElement(
{RequirementKind::SameType, sugaredFirstType, secondType}, loc));
recordedErrors = true;
return true;
}

if (firstType->isTypeParameter() && secondType->isTypeParameter()) {
result.emplace_back(RequirementKind::SameType,
sugaredFirstType, secondType);
Expand Down
5 changes: 5 additions & 0 deletions lib/AST/SubstitutionMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,11 @@ SubstitutionMap SubstitutionMap::get(GenericSignature genericSig,

// Record the replacement.
Type replacement = Type(gp).subst(subs, lookupConformance);

assert((!replacement || replacement->hasError() ||
gp->isParameterPack() == replacement->is<PackType>()) &&
"replacement for pack parameter must be a pack type");

replacementTypes.push_back(replacement);
});

Expand Down
21 changes: 21 additions & 0 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4502,6 +4502,24 @@ operator()(CanType dependentType, Type conformingReplacementType,
ProtocolConformanceRef MakeAbstractConformanceForGenericType::
operator()(CanType dependentType, Type conformingReplacementType,
ProtocolDecl *conformedProtocol) const {
// The places that use this can also produce conformance packs, generally
// just for singleton pack expansions.
if (auto conformingPack = conformingReplacementType->getAs<PackType>()) {
SmallVector<ProtocolConformanceRef, 4> conformances;
for (auto conformingPackElt : conformingPack->getElementTypes()) {
// Look through pack expansions; there's no equivalent conformance
// expansion right now.
auto expansion = conformingPackElt->getAs<PackExpansionType>();
if (expansion) conformingPackElt = expansion->getPatternType();

auto conformance =
(*this)(dependentType, conformingPackElt, conformedProtocol);
conformances.push_back(conformance);
}
return ProtocolConformanceRef(
PackConformance::get(conformingPack, conformedProtocol, conformances));
}

assert((conformingReplacementType->is<ErrorType>() ||
conformingReplacementType->is<SubstitutableType>() ||
conformingReplacementType->is<DependentMemberType>() ||
Expand Down Expand Up @@ -4967,6 +4985,9 @@ TypeBase::getContextSubstitutions(const DeclContext *dc,
else if (genericEnv)
substTy = genericEnv->mapTypeIntoContext(gp);

if (gp->isParameterPack() && !substTy->hasError())
substTy = PackType::getSingletonPackExpansion(substTy);

auto result = substitutions.insert(
{gp->getCanonicalType()->castTo<GenericTypeParamType>(),
substTy});
Expand Down
20 changes: 14 additions & 6 deletions lib/IRGen/GenPack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,18 @@ static llvm::Value *bindWitnessTableAtIndex(IRGenFunction &IGF,
return wtable;
}

/// Find the pack archetype for the given interface type in the given
/// opened element context, which is known to be a forwarding context.
static CanPackArchetypeType
getMappedPackArchetypeType(const OpenedElementContext &context, CanType ty) {
auto packType = cast<PackType>(
context.environment->maybeApplyOuterContextSubstitutions(ty)
->getCanonicalType());
auto archetype = getForwardedPackArchetypeType(packType);
assert(archetype);
return archetype;
}

static void bindElementSignatureRequirementsAtIndex(
IRGenFunction &IGF, OpenedElementContext const &context, llvm::Value *index,
DynamicMetadataRequest request) {
Expand All @@ -275,9 +287,7 @@ static void bindElementSignatureRequirementsAtIndex(
break;
case GenericRequirement::Kind::MetadataPack: {
auto ty = requirement.getTypeParameter();
auto patternPackArchetype = cast<PackArchetypeType>(
context.environment->maybeApplyOuterContextSubstitutions(ty)
->getCanonicalType());
auto patternPackArchetype = getMappedPackArchetypeType(context, ty);
auto response =
IGF.emitTypeMetadataRef(patternPackArchetype, request);
auto elementArchetype =
Expand All @@ -295,9 +305,7 @@ static void bindElementSignatureRequirementsAtIndex(
case GenericRequirement::Kind::WitnessTablePack: {
auto ty = requirement.getTypeParameter();
auto proto = requirement.getProtocol();
auto patternPackArchetype = cast<PackArchetypeType>(
context.environment->maybeApplyOuterContextSubstitutions(ty)
->getCanonicalType());
auto patternPackArchetype = getMappedPackArchetypeType(context, ty);
auto elementArchetype =
context.environment
->mapPackTypeIntoElementContext(
Expand Down
7 changes: 6 additions & 1 deletion lib/SIL/IR/AbstractionPattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1712,7 +1712,12 @@ class SubstFunctionTypePatternVisitor
auto gp = GenericTypeParamType::get(isParameterPack, 0, paramIndex,
TC.Context);
substGenericParams.push_back(gp);
substReplacementTypes.push_back(substTy);
if (isParameterPack) {
substReplacementTypes.push_back(
PackType::getSingletonPackExpansion(substTy));
} else {
substReplacementTypes.push_back(substTy);
}

if (auto layout = pattern.getLayoutConstraint()) {
// Look at the layout constraint on this position in the abstraction pattern
Expand Down
8 changes: 6 additions & 2 deletions lib/SILGen/SILGenConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,13 @@ static SubstitutionMap getSubstitutionsForPropertyInitializer(
// replacement types are the archetypes of the initializer itself.
return SubstitutionMap::get(
nominal->getGenericSignatureOfContext(),
[&](SubstitutableType *type) {
[&](SubstitutableType *type) -> Type {
if (auto gp = type->getAs<GenericTypeParamType>()) {
return genericEnv->mapTypeIntoContext(gp);
auto archetype = genericEnv->mapTypeIntoContext(gp);
if (!gp->isParameterPack())
return archetype;

return PackType::getSingletonPackExpansion(archetype);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slavapestov I tried changing this to:

    return SubstitutionMap::get(
      nominal->getGenericSignatureOfContext(),
      genericEnv->getForwardingSubstitutionMap());

but it still crashed when building the standard library. I didn't spend much time figuring out why because I wanted to finish the PackType representation change, but I can come back to it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, no worries

}

return Type(type);
Expand Down
8 changes: 6 additions & 2 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,12 @@ Solution::computeSubstitutions(GenericSignature sig,
return SubstitutionMap();

TypeSubstitutionMap subs;
for (const auto &opened : openedTypes->second)
subs[opened.first] = getFixedType(opened.second);
for (const auto &opened : openedTypes->second) {
auto type = getFixedType(opened.second);
if (opened.first->isParameterPack() && !type->is<PackType>())
type = PackType::getSingletonPackExpansion(type);
subs[opened.first] = type;
}

auto lookupConformanceFn =
[&](CanType original, Type replacement,
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1501,6 +1501,7 @@ void PotentialBindings::infer(Constraint *constraint) {
break;

auto elementType = elementEnv->mapPackTypeIntoElementContext(packType);
assert(!elementType->is<PackType>());
addPotentialBinding({elementType, AllowedBindingKind::Exact, constraint});

break;
Expand Down
1 change: 0 additions & 1 deletion lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,6 @@ namespace {
ConstraintSystem &getConstraintSystem() const { return CS; }

void addPackElementEnvironment(PackExpansionExpr *expr) {
CS.addPackElementEnvironment(expr);
PackElementEnvironments.push_back(expr);
}

Expand Down
18 changes: 18 additions & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8783,6 +8783,19 @@ ConstraintSystem::simplifyBindTupleOfFunctionParamsConstraint(
return SolutionKind::Solved;
}

static Type lookThroughSingletonPackExpansion(Type ty) {
if (auto pack = ty->getAs<PackType>()) {
if (pack->getNumElements() == 1) {
if (auto expansion = pack->getElementType(0)->getAs<PackExpansionType>()) {
auto countType = expansion->getCountType();
if (countType->isEqual(expansion->getPatternType()))
return countType;
}
}
}
return ty;
}

ConstraintSystem::SolutionKind
ConstraintSystem::simplifyPackElementOfConstraint(Type first, Type second,
TypeMatchOptions flags,
Expand All @@ -8802,6 +8815,11 @@ ConstraintSystem::simplifyPackElementOfConstraint(Type first, Type second,
return SolutionKind::Solved;
}

// FIXME: I'm not sure this is actually necessary; I may only be seeing
// this because of something I've screwed up in element generic
// environments.
elementType = lookThroughSingletonPackExpansion(elementType);

// This constraint only exists to vend bindings.
auto *packEnv = DC->getGenericEnvironmentOfContext();

Expand Down
24 changes: 13 additions & 11 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -649,29 +649,31 @@ std::pair<Type, OpenedArchetypeType *> ConstraintSystem::openExistentialType(
return {result, opened};
}

void ConstraintSystem::addPackElementEnvironment(PackExpansionExpr *expr) {
auto *locator = getConstraintLocator(expr);
PackExpansionEnvironments[locator] = UUID::fromTime();
}

GenericEnvironment *
ConstraintSystem::getPackElementEnvironment(ConstraintLocator *locator,
CanType shapeClass) {
assert(locator->directlyAt<PackExpansionExpr>());

std::pair<UUID, Type> uuidAndShape;
auto result = PackExpansionEnvironments.find(locator);
if (result == PackExpansionEnvironments.end())
return nullptr;
if (result == PackExpansionEnvironments.end()) {
uuidAndShape = std::make_pair(UUID::fromTime(), shapeClass);
PackExpansionEnvironments[locator] = uuidAndShape;
} else {
uuidAndShape = result->second;
}

if (!shapeClass->is<PackArchetypeType>())
if (!shapeClass->is<PackArchetypeType>() ||
!shapeClass->isEqual(uuidAndShape.second))
return nullptr;

auto uuid = result->second;
auto &ctx = getASTContext();
auto elementSig = ctx.getOpenedElementSignature(
DC->getGenericSignatureOfContext().getCanonicalSignature(), shapeClass);
auto *contextEnv = DC->getGenericEnvironmentOfContext();
auto contextSubs = contextEnv->getForwardingSubstitutionMap();
return GenericEnvironment::forOpenedElement(elementSig, uuid, shapeClass,
contextSubs);
return GenericEnvironment::forOpenedElement(elementSig, uuidAndShape.first,
shapeClass, contextSubs);
}

/// Extend the given depth map by adding depths for all of the subexpressions
Expand Down
Loading