Skip to content

GenericSignatureImpl, #31712: Plug remaining relevant methods with ty… #31927

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.

Sign up for GitHub

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 1 commit into from
May 21, 2020
Merged

GenericSignatureImpl, #31712: Plug remaining relevant methods with ty… #31927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 2 additions & 2 deletions include/swift/AST/GenericSignature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,12 @@ class alignas(1 << TypeAlignInBits) GenericSignatureImpl final
/// Determine whether the given dependent type is equal to a concrete type.
bool isConcreteType(Type type) const;

/// Return the concrete type that the given dependent type is constrained to,
/// Return the concrete type that the given type parameter is constrained to,
/// or the null Type if it is not the subject of a concrete same-type
/// constraint.
Type getConcreteType(Type type) const;

/// Return the layout constraint that the given dependent type is constrained
/// Return the layout constraint that the given type parameter is constrained
/// to, or the null LayoutConstraint if it is not the subject of layout
/// constraint.
LayoutConstraint getLayoutConstraint(Type type) const;
Expand Down
9 changes: 5 additions & 4 deletions lib/AST/GenericSignature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ Type GenericSignatureImpl::getSuperclassBound(Type type) const {
/// required to conform.
GenericSignature::RequiredProtocols
GenericSignatureImpl::getRequiredProtocols(Type type) const {
if (!type->isTypeParameter()) return { };
assert(type->isTypeParameter() && "Expected a type parameter");
Copy link
Contributor

@slavapestov slavapestov May 20, 2020

Choose a reason for hiding this comment

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

It would also be nice to assert if resolveEquivalenceClass() returns an unresolved type or concrete type, instead of returning an empty array, but I don't know what the fallout from that might be.

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis May 20, 2020

Choose a reason for hiding this comment

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

Do you think calling this on a same-type-to-concrete type parameter and getting back an empty array might backfire on us?

Copy link
Contributor

@slavapestov slavapestov May 21, 2020

Choose a reason for hiding this comment

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

I would expect that any code doing so is already incorrect, because we can't hope to return a consistent result in this case. Should it be empty, or reflect any existing (possibly redundant) conformance constraints?

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis May 21, 2020

Choose a reason for hiding this comment

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

I see. There is a similar issue with getSuperclassBound, and others, I guess. For example, in checkTypeWitness, we always want an explicit bound, not one implied by a conformance constraint, to avoid checking both inheritance and conformance (a syntactic conformance, even if it's broken, should always be enough for the candidate to pass).


auto &builder = *getGenericSignatureBuilder();
auto equivClass =
Expand Down Expand Up @@ -479,11 +479,11 @@ bool GenericSignatureImpl::isConcreteType(Type type) const {
return bool(getConcreteType(type));
}

/// Return the concrete type that the given dependent type is constrained to,
/// Return the concrete type that the given type parameter is constrained to,
/// or the null Type if it is not the subject of a concrete same-type
/// constraint.
Type GenericSignatureImpl::getConcreteType(Type type) const {
if (!type->isTypeParameter()) return Type();
assert(type->isTypeParameter() && "Expected a type parameter");

auto &builder = *getGenericSignatureBuilder();
auto equivClass =
Expand All @@ -496,7 +496,8 @@ Type GenericSignatureImpl::getConcreteType(Type type) const {
}

LayoutConstraint GenericSignatureImpl::getLayoutConstraint(Type type) const {
if (!type->isTypeParameter()) return LayoutConstraint();
assert(type->isTypeParameter() &&
"Only type parameters can have layout constraints");

auto &builder = *getGenericSignatureBuilder();
auto equivClass =
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/SubstitutionMap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ Type SubstitutionMap::lookupSubstitution(CanSubstitutableType type) const {

// The generic parameter may have been made concrete by the generic signature,
// substitute into the concrete type.
if (auto concreteType = genericSig->getConcreteType(genericParam)){
if (auto concreteType = genericSig->getConcreteType(genericParam)) {
// Set the replacement type to an error, to block infinite recursion.
replacementType = ErrorType::get(concreteType);

Expand Down