Skip to content

Remove validateDeclForNameLookup and related logic #24957

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

Closed
wants to merge 9 commits into from
Closed

Remove validateDeclForNameLookup and related logic #24957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a268e0c
Move generic signature debugging code that triggered extra work too e…
xymus May 17, 2019
da6cf5e
TypeAliasDecl::getStructuralType returns a validated type if available
xymus Jul 24, 2019
d2e3df4
TypeChecker: substMemberTypeWithBase respects structural type validation
xymus Jul 18, 2019
92bec03
TypeChecker: resolveTypeDecl use the structural type of alias decls
xymus Jul 24, 2019
da4a38c
Sema: StructuralTypeRequest wraps the resolved type in a TypeAliasType
xymus Aug 1, 2019
95a8d08
Sema: print type alias underlying types when debugging generics
xymus Aug 12, 2019
7476430
Remove validateDeclForNameLookup and related logic
xymus Aug 20, 2019
d81a5d6
Remove resolveProtocolEnvironment, it should be no more needed
xymus Jun 17, 2019
87be387
Sema: validateDecl validates the context of GenericTypeParamDecls
xymus Aug 12, 2019
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: 0 additions & 3 deletions include/swift/AST/LazyResolver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ class LazyResolver {
/// consistency and provides the value a type.
virtual void resolveDeclSignature(ValueDecl *VD) = 0;

/// Resolve the generic environment of the given protocol.
virtual void resolveProtocolEnvironment(ProtocolDecl *proto) = 0;

/// Resolve the type of an extension.
///
/// This can be called to ensure that the members of an extension can be
Expand Down
5 changes: 4 additions & 1 deletion lib/AST/Decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3624,7 +3624,10 @@ UnboundGenericType *TypeAliasDecl::getUnboundGenericType() const {

Type TypeAliasDecl::getStructuralType() const {
assert(!getGenericParams());


if (auto resolved = this->getUnderlyingTypeLoc().getType())
return resolved;

auto &context = getASTContext();
return evaluateOrDefault(context.evaluator,
StructuralTypeRequest { const_cast<TypeAliasDecl *>(this) },
Expand Down
3 changes: 0 additions & 3 deletions lib/AST/GenericSignatureBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3785,9 +3785,6 @@ PotentialArchetype *GenericSignatureBuilder::realizePotentialArchetype(

static Type getStructuralType(TypeDecl *typeDecl) {
if (auto typealias = dyn_cast<TypeAliasDecl>(typeDecl)) {
if (auto resolved = typealias->getUnderlyingTypeLoc().getType())
return resolved;

return typealias->getStructuralType();
}

Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/CalleeCandidateInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ void CalleeCandidateInfo::collectCalleeCandidates(Expr *fn,
CS.DC, instanceType, NameLookupFlags::IgnoreAccessControl);
for (auto ctor : ctors) {
if (!ctor.getValueDecl()->hasInterfaceType())
CS.getTypeChecker().validateDeclForNameLookup(ctor.getValueDecl());
CS.getTypeChecker().validateDecl(ctor.getValueDecl());
if (ctor.getValueDecl()->hasInterfaceType())
candidates.push_back({ ctor.getValueDecl(), 1 });
}
Expand Down
4 changes: 2 additions & 2 deletions lib/Sema/TypeCheckAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@ bool swift::isValidDynamicCallableMethod(FuncDecl *decl, DeclContext *DC,
// `ExpressibleByStringLiteral`.
// `D.Value` and the return type can be arbitrary.

TC.validateDeclForNameLookup(decl);
TC.validateDecl(decl);
auto paramList = decl->getParameters();
if (paramList->size() != 1 || paramList->get(0)->isVariadic()) return false;
auto argType = paramList->get(0)->getType();
Expand Down Expand Up @@ -1235,7 +1235,7 @@ visitDynamicMemberLookupAttr(DynamicMemberLookupAttr *attr) {
auto oneCandidate = candidates.front();
candidates.filter([&](LookupResultEntry entry, bool isOuter) -> bool {
auto cand = cast<SubscriptDecl>(entry.getValueDecl());
TC.validateDeclForNameLookup(cand);
TC.validateDecl(cand);
return isValidDynamicMemberLookupSubscript(cand, decl, TC);
});

Expand Down
132 changes: 18 additions & 114 deletions lib/Sema/TypeCheckDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2912,6 +2912,16 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
TC.inferDefaultWitnesses(PD);

if (TC.Context.LangOpts.DebugGenericSignatures) {
auto *sig = PD->getGenericSignature();
PD->printContext(llvm::errs());
llvm::errs() << "\n";
llvm::errs() << "Generic signature: ";
sig->print(llvm::errs());
llvm::errs() << "\n";
llvm::errs() << "Canonical generic signature: ";
sig->getCanonicalSignature()->print(llvm::errs());
llvm::errs() << "\n";

auto requirementsSig =
GenericSignature::get({PD->getProtocolSelfType()},
PD->getRequirementSignature());
Expand All @@ -2920,7 +2930,10 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
PD->dumpRef(llvm::errs());
llvm::errs() << "\n";
llvm::errs() << "Requirement signature: ";
requirementsSig->print(llvm::errs());

PrintOptions opts;
opts.PrintTypeAliasUnderlyingType = true;
requirementsSig->print(llvm::errs(), opts);
llvm::errs() << "\n";

// Note: One cannot canonicalize a requirement signature, because
Expand Down Expand Up @@ -3627,10 +3640,6 @@ static Type buildAddressorResultType(TypeChecker &TC,
}

void TypeChecker::validateDecl(ValueDecl *D) {
// Generic parameters are validated as part of their context.
if (isa<GenericTypeParamDecl>(D))
return;

// Handling validation failure due to re-entrancy is left
// up to the caller, who must call hasValidSignature() to
// check that validateDecl() returned a fully-formed decl.
Expand Down Expand Up @@ -3668,6 +3677,10 @@ void TypeChecker::validateDecl(ValueDecl *D) {
return;
}

// Generic parameters are validated as part of their context.
if (isa<GenericTypeParamDecl>(D))
return;

// Validating the parent may have triggered validation of this declaration,
// so just return if that was the case.
if (D->hasValidationStarted()) {
Expand Down Expand Up @@ -3759,33 +3772,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
validateGenericTypeSignature(proto);
proto->setSignatureIsValidated();

// See the comment in validateDeclForNameLookup(); we may have validated
// the alias before we built the protocol's generic environment.
//
// FIXME: Hopefully this can all go away with the ITC.
for (auto member : proto->getMembers()) {
if (auto *aliasDecl = dyn_cast<TypeAliasDecl>(member)) {
if (!aliasDecl->isGeneric()) {
aliasDecl->setGenericEnvironment(proto->getGenericEnvironment());

// The generic environment didn't exist until now, we may have
// unresolved types we will need to deal with, and need to record the
// appropriate substitutions for that environment. Wipe out the types
// and validate them again.
aliasDecl->getUnderlyingTypeLoc().setType(Type());
aliasDecl->setInterfaceType(Type());

// Check generic parameters, if needed.
if (aliasDecl->hasValidationStarted()) {
validateTypealiasType(*this, aliasDecl);
} else {
DeclValidationRAII IBV(aliasDecl);
validateTypealiasType(*this, aliasDecl);
}
}
}
}

break;
}

Expand Down Expand Up @@ -4090,88 +4076,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
assert(D->hasValidSignature());
}

void TypeChecker::validateDeclForNameLookup(ValueDecl *D) {
// Validate the context.
auto dc = D->getDeclContext();
if (auto nominal = dyn_cast<NominalTypeDecl>(dc)) {
validateDeclForNameLookup(nominal);
if (!nominal->hasInterfaceType())
return;
} else if (auto ext = dyn_cast<ExtensionDecl>(dc)) {
validateExtension(ext);
if (!ext->hasValidSignature())
return;
}

switch (D->getKind()) {
case DeclKind::Protocol: {
auto proto = cast<ProtocolDecl>(D);
if (proto->hasInterfaceType())
return;
proto->computeType();

break;
}
case DeclKind::AssociatedType: {
auto assocType = cast<AssociatedTypeDecl>(D);
if (assocType->hasInterfaceType())
return;
assocType->computeType();
break;
}
case DeclKind::TypeAlias: {
auto typealias = cast<TypeAliasDecl>(D);
if (typealias->getUnderlyingTypeLoc().getType())
return;

// Perform earlier validation of typealiases in protocols.
if (isa<ProtocolDecl>(dc)) {
if (!typealias->getGenericParams()) {
if (typealias->isBeingValidated()) return;

auto helper = [&] {
TypeResolutionOptions options(
(typealias->getGenericParams() ?
TypeResolverContext::GenericTypeAliasDecl :
TypeResolverContext::TypeAliasDecl));
auto &underlyingTL = typealias->getUnderlyingTypeLoc();
if (underlyingTL.isNull() ||
validateType(underlyingTL,
TypeResolution::forStructural(typealias), options)) {
typealias->setInvalid();
underlyingTL.setInvalidType(Context);
}

typealias->setUnderlyingType(underlyingTL.getType());

// Note that this doesn't set the generic environment of the alias yet,
// because we haven't built one for the protocol.
//
// See how validateDecl() sets the generic environment on alias members
// explicitly.
//
// FIXME: Hopefully this can all go away with the ITC.
};

if (typealias->hasValidationStarted()) {
helper();
} else {
DeclValidationRAII IBV(typealias);
helper();
}

return;
}
}
LLVM_FALLTHROUGH;
}

default:
validateDecl(D);
break;
}
}

llvm::Expected<DeclRange>
EmittedMembersRequest::evaluate(Evaluator &evaluator,
ClassDecl *CD) const {
Expand Down
30 changes: 13 additions & 17 deletions lib/Sema/TypeCheckGeneric.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -759,22 +759,6 @@ void TypeChecker::validateGenericTypeSignature(GenericTypeDecl *typeDecl) {
// The generic signature and environment is created lazily by
// GenericContext::getGenericSignature(), so there is nothing we
// need to do.

// Debugging of the generic signature builder and generic signature
// generation.
if (Context.LangOpts.DebugGenericSignatures) {
auto *sig = proto->getGenericSignature();

proto->printContext(llvm::errs());
llvm::errs() << "\n";
llvm::errs() << "Generic signature: ";
sig->print(llvm::errs());
llvm::errs() << "\n";
llvm::errs() << "Canonical generic signature: ";
sig->getCanonicalSignature()->print(llvm::errs());
llvm::errs() << "\n";
}

return;
}

Expand Down Expand Up @@ -1046,5 +1030,17 @@ swift::StructuralTypeRequest::evaluate(Evaluator &evaluator,

auto typeRepr = D->getUnderlyingTypeLoc().getTypeRepr();
auto resolution = TypeResolution::forStructural(D);
return resolution.resolveType(typeRepr, options);
auto resolvedType = resolution.resolveType(typeRepr, options);

// Wrap the resolved type in a TypeAliasType
auto *genericSig = D->getGenericSignature();
SubstitutionMap subs;
if (genericSig)
subs = genericSig->getIdentitySubstitutionMap();

Type parent;
auto parentDC = D->getDeclContext();
if (parentDC->isTypeContext())
parent = parentDC->getSelfInterfaceType();
return TypeAliasType::get(D, parent, subs, resolvedType);
}
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckPattern.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1497,7 +1497,7 @@ bool TypeChecker::coercePatternToType(Pattern *&P, TypeResolution resolution,
}

// If there is a subpattern, push the enum element type down onto it.
validateDeclForNameLookup(elt);
validateDecl(elt);
if (EEP->hasSubPattern()) {
Pattern *sub = EEP->getSubPattern();
if (!elt->hasAssociatedValues()) {
Expand Down
40 changes: 22 additions & 18 deletions lib/Sema/TypeCheckType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,10 +645,11 @@ Type TypeChecker::resolveTypeInContext(
}
}
}

// Finally, substitute the base type into the member type.
return substMemberTypeWithBase(fromDC->getParentModule(), typeDecl,
selfType, resolution.usesArchetypes());
selfType, resolution.usesArchetypes(),
resolution.getStage());
}

static TypeResolutionOptions
Expand Down Expand Up @@ -978,9 +979,18 @@ static Type resolveTypeDecl(TypeDecl *typeDecl, SourceLoc loc,
auto &diags = ctx.Diags;
auto lazyResolver = ctx.getLazyResolver();

Type type = Type();

// Use only the structural type of an alias decl in structural mode
auto aliasDecl = dyn_cast<TypeAliasDecl>(typeDecl);
if (resolution.getStage() == TypeResolutionStage::Structural &&
aliasDecl && !aliasDecl->getGenericParams() &&
!aliasDecl->getUnderlyingTypeLoc().wasValidated()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a similar check in getStructuralType() in GenericSignatureBuilder.cpp. Maybe TypeAliasDecl::getStructuralType() should automatically return the checked underlying type if its available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've applied the suggested change to TypeAliasDecl::getStructuralType. I'll leave this check here because we still want to execute TypeChecker::resolveTypeInContext if the type was previously validated, it provides better diagnostics.

type = aliasDecl->getStructuralType();

// Don't validate nominal type declarations during extension binding.
if (!options.is(TypeResolverContext::ExtensionBinding) ||
!isa<NominalTypeDecl>(typeDecl)) {
} else if (!options.is(TypeResolverContext::ExtensionBinding) ||
!isa<NominalTypeDecl>(typeDecl)) {
// Validate the declaration.
if (lazyResolver)
lazyResolver->resolveDeclSignature(typeDecl);
Expand All @@ -997,9 +1007,10 @@ static Type resolveTypeDecl(TypeDecl *typeDecl, SourceLoc loc,

// Resolve the type declaration to a specific type. How this occurs
// depends on the current context and where the type was found.
Type type =
TypeChecker::resolveTypeInContext(typeDecl, foundDC, resolution, options,
generic);
if (!type)
type =
TypeChecker::resolveTypeInContext(typeDecl, foundDC, resolution,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should getting the structural type be pushed down into resolveTypeInContext(), since it has access to the resolution object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We probably could, but we also need to avoid the call to the previous resolveDeclSignature because it triggers validateDecl.

options, generic);

if (type->is<UnboundGenericType>() && !generic &&
!options.is(TypeResolverContext::TypeAliasDecl) &&
Expand Down Expand Up @@ -3362,7 +3373,8 @@ Type TypeResolver::buildProtocolType(
Type TypeChecker::substMemberTypeWithBase(ModuleDecl *module,
TypeDecl *member,
Type baseTy,
bool useArchetypes) {
bool useArchetypes,
TypeResolutionStage stage) {
Type sugaredBaseTy = baseTy;

// For type members of a base class, make sure we use the right
Expand Down Expand Up @@ -3410,20 +3422,12 @@ Type TypeChecker::substMemberTypeWithBase(ModuleDecl *module,
aliasDecl, baseTy,
aliasDecl->getASTContext());
}

// FIXME: If this is a protocol typealias and we haven't built the
// protocol's generic environment yet, do so now, to ensure the
// typealias's underlying type has fully resolved dependent
// member types.
if (auto *protoDecl = dyn_cast<ProtocolDecl>(aliasDecl->getDeclContext())) {
ASTContext &ctx = protoDecl->getASTContext();
ctx.getLazyResolver()->resolveProtocolEnvironment(protoDecl);
}
}

Type resultType;
auto memberType = aliasDecl ? aliasDecl->getUnderlyingTypeLoc().getType()
auto memberType = aliasDecl ? aliasDecl->getStructuralType()
: member->getDeclaredInterfaceType();

SubstitutionMap subs;
if (baseTy) {
// Cope with the presence of unbound generic types, which are ill-formed
Expand Down
Loading