Skip to content

[Property wrappers] Fix source compatibility issue with attribute lookup #27645

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
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
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsCommon.def
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ ERROR(circular_protocol_def,none,
NOTE(kind_declname_declared_here,none,
"%0 %1 declared here", (DescriptiveDeclKind, DeclName))

WARNING(warn_property_wrapper_module_scope,none,
"ignoring associated type %0 in favor of module-scoped property "
"wrapper %0; please qualify the reference with %1",
(DeclName, Identifier))

#ifndef DIAG_NO_UNDEF
# if defined(DIAG)
# undef DIAG
Expand Down
71 changes: 69 additions & 2 deletions lib/AST/NameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,13 @@ void namelookup::recordLookupOfTopLevelName(DeclContext *topLevelContext,
nameTracker->addTopLevelName(name.getBaseName(), isCascading);
}

namespace {
/// Whether we're looking up outer results or not.
enum class LookupOuterResults {
Excluded,
Included
};
}

/// Retrieve the set of type declarations that are directly referenced from
/// the given parsed type representation.
Expand Down Expand Up @@ -1878,11 +1885,15 @@ resolveTypeDeclsToNominal(Evaluator &evaluator,
/// Perform unqualified name lookup for types at the given location.
static DirectlyReferencedTypeDecls
directReferencesForUnqualifiedTypeLookup(DeclName name,
SourceLoc loc, DeclContext *dc) {
SourceLoc loc, DeclContext *dc,
LookupOuterResults lookupOuter) {
DirectlyReferencedTypeDecls results;
UnqualifiedLookup::Options options =
UnqualifiedLookup::Flags::TypeLookup |
UnqualifiedLookup::Flags::AllowProtocolMembers;
if (lookupOuter == LookupOuterResults::Included)
options |= UnqualifiedLookup::Flags::IncludeOuterResults;

UnqualifiedLookup lookup(name, dc, loc, options);
for (const auto &result : lookup.Results) {
if (auto typeDecl = dyn_cast<TypeDecl>(result.getValueDecl()))
Expand Down Expand Up @@ -1957,7 +1968,8 @@ directReferencesForIdentTypeRepr(Evaluator &evaluator,
current =
directReferencesForUnqualifiedTypeLookup(component->getIdentifier(),
component->getIdLoc(),
dc);
dc,
LookupOuterResults::Excluded);

// If we didn't find anything, fail now.
if (current.empty())
Expand Down Expand Up @@ -2195,6 +2207,19 @@ ExtendedNominalRequest::evaluate(Evaluator &evaluator,
return nominalTypes.empty() ? nullptr : nominalTypes[0];
}

/// Whether there are only associated types in the set of declarations.
static bool declsAreAssociatedTypes(ArrayRef<TypeDecl *> decls) {
if (decls.empty())
return false;

for (auto decl : decls) {
if (!isa<AssociatedTypeDecl>(decl))
return false;
}

return true;
}

llvm::Expected<NominalTypeDecl *>
CustomAttrNominalRequest::evaluate(Evaluator &evaluator,
CustomAttr *attr, DeclContext *dc) const {
Expand All @@ -2217,6 +2242,48 @@ CustomAttrNominalRequest::evaluate(Evaluator &evaluator,
if (nominals.size() == 1 && !isa<ProtocolDecl>(nominals.front()))
return nominals.front();

// If we found declarations that are associated types, look outside of
// the current context to see if we can recover.
if (declsAreAssociatedTypes(decls)) {
if (auto typeRepr = typeLoc.getTypeRepr()) {
if (auto identTypeRepr = dyn_cast<SimpleIdentTypeRepr>(typeRepr)) {
auto assocType = cast<AssociatedTypeDecl>(decls.front());

modulesFound.clear();
anyObject = false;
decls = directReferencesForUnqualifiedTypeLookup(
identTypeRepr->getIdentifier(), identTypeRepr->getIdLoc(), dc,
LookupOuterResults::Included);
nominals = resolveTypeDeclsToNominal(evaluator, ctx, decls,
modulesFound, anyObject);
if (nominals.size() == 1 && !isa<ProtocolDecl>(nominals.front())) {
auto nominal = nominals.front();
if (nominal->getDeclContext()->isModuleScopeContext()) {
// Complain, producing module qualification in a Fix-It.
auto moduleName = nominal->getParentModule()->getName();
ctx.Diags.diagnose(typeRepr->getLoc(),
diag::warn_property_wrapper_module_scope,
identTypeRepr->getIdentifier(),
moduleName)
.fixItInsert(typeRepr->getLoc(),
moduleName.str().str() + ".");
ctx.Diags.diagnose(assocType, diag::kind_declname_declared_here,
assocType->getDescriptiveKind(),
assocType->getFullName());

ComponentIdentTypeRepr *components[2] = {
new (ctx) SimpleIdentTypeRepr(typeRepr->getLoc(), moduleName),
identTypeRepr
};

typeLoc = TypeLoc(IdentTypeRepr::create(ctx, components));
return nominal;
}
}
}
}
}

return nullptr;
}

Expand Down
10 changes: 10 additions & 0 deletions test/decl/var/property_wrappers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1779,3 +1779,13 @@ struct SR_11477_W3 { // Okay
get { return 0 }
}
}

// rdar://problem/56213175 - backward compatibility issue with Swift 5.1,
// which unconditionally skipped protocol members.
protocol ProtocolWithWrapper {
associatedtype Wrapper = Float // expected-note{{associated type 'Wrapper' declared here}}
}

struct UsesProtocolWithWrapper: ProtocolWithWrapper {
@Wrapper var foo: Int // expected-warning{{ignoring associated type 'Wrapper' in favor of module-scoped property wrapper 'Wrapper'; please qualify the reference with 'property_wrappers'}}{{4-4=property_wrappers.}}
}