Skip to content

Commit 5d6f694

Browse files
authored
Merge pull request #75158 from tshortli/member-import-visibility-package
AST: Add a `IgnoreMissingImports` option to name lookup
2 parents a55beaa + d002da0 commit 5d6f694

20 files changed

+162
-66
lines changed

include/swift/AST/LookupKinds.h

+4
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ enum NLOptions : unsigned {
6060
/// This lookup should only return macro declarations.
6161
NL_OnlyMacros = 1 << 8,
6262

63+
/// Include members that would otherwise be filtered out because they come
64+
/// from a module that has not been imported.
65+
NL_IgnoreMissingImports = 1 << 9,
66+
6367
/// The default set of options used for qualified name lookup.
6468
///
6569
/// FIXME: Eventually, add NL_ProtocolMembers to this, once all of the

include/swift/AST/NameLookup.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,10 @@ enum class UnqualifiedLookupFlags {
249249
ModuleLookup = 1 << 8,
250250
/// This lookup should discard 'Self' requirements in protocol extension
251251
/// 'where' clauses.
252-
DisregardSelfBounds = 1 << 9
252+
DisregardSelfBounds = 1 << 9,
253+
/// This lookup should include members that would otherwise be filtered out
254+
/// because they come from a module that has not been imported.
255+
IgnoreMissingImports = 1 << 10,
253256
};
254257

255258
using UnqualifiedLookupOptions = OptionSet<UnqualifiedLookupFlags>;

include/swift/Sema/CSFix.h

+7-3
Original file line numberDiff line numberDiff line change
@@ -1923,11 +1923,14 @@ class MoveOutOfOrderArgument final : public ConstraintFix {
19231923
};
19241924

19251925
class AllowInaccessibleMember final : public AllowInvalidMemberRef {
1926+
bool IsMissingImport;
1927+
19261928
AllowInaccessibleMember(ConstraintSystem &cs, Type baseType,
19271929
ValueDecl *member, DeclNameRef name,
1928-
ConstraintLocator *locator)
1930+
ConstraintLocator *locator, bool isMissingImport)
19291931
: AllowInvalidMemberRef(cs, FixKind::AllowInaccessibleMember, baseType,
1930-
member, name, locator) {}
1932+
member, name, locator),
1933+
IsMissingImport(isMissingImport) {}
19311934

19321935
public:
19331936
std::string getName() const override {
@@ -1942,7 +1945,8 @@ class AllowInaccessibleMember final : public AllowInvalidMemberRef {
19421945

19431946
static AllowInaccessibleMember *create(ConstraintSystem &cs, Type baseType,
19441947
ValueDecl *member, DeclNameRef name,
1945-
ConstraintLocator *locator);
1948+
ConstraintLocator *locator,
1949+
bool isMissingImport);
19461950

19471951
static bool classof(const ConstraintFix *fix) {
19481952
return fix->getKind() == FixKind::AllowInaccessibleMember;

include/swift/Sema/ConstraintSystem.h

+4
Original file line numberDiff line numberDiff line change
@@ -1975,6 +1975,10 @@ struct MemberLookupResult {
19751975
/// The member is inaccessible (e.g. a private member in another file).
19761976
UR_Inaccessible,
19771977

1978+
/// The member is not visible because it comes from a module that was not
1979+
/// imported.
1980+
UR_MissingImport,
1981+
19781982
/// This is a `WritableKeyPath` being used to look up read-only member,
19791983
/// used in situations involving dynamic member lookup via keypath,
19801984
/// because it's not known upfront what access capability would the

lib/AST/NameLookup.cpp

+10-4
Original file line numberDiff line numberDiff line change
@@ -2363,9 +2363,11 @@ static bool isAcceptableLookupResult(const DeclContext *dc,
23632363
if (!decl->isAccessibleFrom(dc, /*forConformance*/ false,
23642364
allowUsableFromInline))
23652365
return false;
2366+
}
23662367

2367-
// Check that there is some import in the originating context that
2368-
// makes this decl visible.
2368+
// Check that there is some import in the originating context that makes this
2369+
// decl visible.
2370+
if (!(options & NL_IgnoreMissingImports)) {
23692371
if (missingExplicitImportForMemberDecl(dc, decl))
23702372
return false;
23712373
}
@@ -3983,7 +3985,8 @@ bool IsCallAsFunctionNominalRequest::evaluate(Evaluator &evaluator,
39833985
// that will be checked when we actually try to solve with a `callAsFunction`
39843986
// member access.
39853987
SmallVector<ValueDecl *, 4> results;
3986-
auto opts = NL_QualifiedDefault | NL_ProtocolMembers | NL_IgnoreAccessControl;
3988+
auto opts = NL_QualifiedDefault | NL_ProtocolMembers |
3989+
NL_IgnoreAccessControl | NL_IgnoreMissingImports;
39873990
dc->lookupQualified(decl, DeclNameRef(ctx.Id_callAsFunction),
39883991
decl->getLoc(), opts, results);
39893992

@@ -4138,8 +4141,11 @@ void swift::simple_display(llvm::raw_ostream &out, NLOptions options) {
41384141
FLAG(NL_RemoveOverridden)
41394142
FLAG(NL_IgnoreAccessControl)
41404143
FLAG(NL_OnlyTypes)
4141-
FLAG(NL_OnlyMacros)
41424144
FLAG(NL_IncludeAttributeImplements)
4145+
FLAG(NL_IncludeUsableFromInline)
4146+
FLAG(NL_ExcludeMacroExpansions)
4147+
FLAG(NL_OnlyMacros)
4148+
FLAG(NL_IgnoreMissingImports)
41434149
#undef FLAG
41444150
};
41454151

lib/AST/UnqualifiedLookup.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,8 @@ NLOptions UnqualifiedLookupFactory::computeBaseNLOptions(
596596
baseNLOptions |= NL_OnlyMacros;
597597
if (options.contains(Flags::IgnoreAccessControl))
598598
baseNLOptions |= NL_IgnoreAccessControl;
599+
if (options.contains(Flags::IgnoreMissingImports))
600+
baseNLOptions |= NL_IgnoreMissingImports;
599601
return baseNLOptions;
600602
}
601603

lib/ClangImporter/ClangImporter.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -6076,14 +6076,15 @@ static void lookupRelatedFuncs(AbstractFunctionDecl *func,
60766076
else
60776077
swiftName = func->getName();
60786078

6079+
NLOptions options = NL_IgnoreAccessControl | NL_IgnoreMissingImports;
60796080
if (auto ty = func->getDeclContext()->getSelfNominalTypeDecl()) {
60806081
ty->lookupQualified({ ty }, DeclNameRef(swiftName), func->getLoc(),
6081-
NL_QualifiedDefault | NL_IgnoreAccessControl, results);
6082+
NL_QualifiedDefault | options, results);
60826083
}
60836084
else {
60846085
auto mod = func->getDeclContext()->getParentModule();
60856086
mod->lookupQualified(mod, DeclNameRef(swiftName), func->getLoc(),
6086-
NL_RemoveOverridden | NL_IgnoreAccessControl, results);
6087+
NL_RemoveOverridden | options, results);
60876088
}
60886089
}
60896090

lib/Sema/CSDiagnostics.cpp

+4-3
Original file line numberDiff line numberDiff line change
@@ -6187,11 +6187,12 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
61876187
}
61886188

61896189
auto loc = nameLoc.isValid() ? nameLoc.getStartLoc() : ::getLoc(anchor);
6190-
auto accessLevel = Member->getFormalAccessScope().accessLevelForDiagnostics();
6191-
if (accessLevel == AccessLevel::Public &&
6192-
diagnoseMissingImportForMember(Member, getDC(), loc))
6190+
if (IsMissingImport) {
6191+
diagnoseMissingImportForMember(Member, getDC(), loc);
61936192
return true;
6193+
}
61946194

6195+
auto accessLevel = Member->getFormalAccessScope().accessLevelForDiagnostics();
61956196
if (auto *CD = dyn_cast<ConstructorDecl>(Member)) {
61966197
emitDiagnosticAt(loc, diag::init_candidate_inaccessible,
61976198
CD->getResultInterfaceType(), accessLevel)

lib/Sema/CSDiagnostics.h

+4-2
Original file line numberDiff line numberDiff line change
@@ -1638,11 +1638,13 @@ class ClosureParamDestructuringFailure final : public FailureDiagnostic {
16381638
/// ```
16391639
class InaccessibleMemberFailure final : public FailureDiagnostic {
16401640
ValueDecl *Member;
1641+
bool IsMissingImport;
16411642

16421643
public:
16431644
InaccessibleMemberFailure(const Solution &solution, ValueDecl *member,
1644-
ConstraintLocator *locator)
1645-
: FailureDiagnostic(solution, locator), Member(member) {}
1645+
ConstraintLocator *locator, bool isMissingImport)
1646+
: FailureDiagnostic(solution, locator), Member(member),
1647+
IsMissingImport(isMissingImport) {}
16461648

16471649
bool diagnoseAsError() override;
16481650
};

lib/Sema/CSFix.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -1152,16 +1152,16 @@ MoveOutOfOrderArgument *MoveOutOfOrderArgument::create(
11521152

11531153
bool AllowInaccessibleMember::diagnose(const Solution &solution,
11541154
bool asNote) const {
1155-
InaccessibleMemberFailure failure(solution, getMember(), getLocator());
1155+
InaccessibleMemberFailure failure(solution, getMember(), getLocator(),
1156+
IsMissingImport);
11561157
return failure.diagnose(asNote);
11571158
}
11581159

1159-
AllowInaccessibleMember *
1160-
AllowInaccessibleMember::create(ConstraintSystem &cs, Type baseType,
1161-
ValueDecl *member, DeclNameRef name,
1162-
ConstraintLocator *locator) {
1163-
return new (cs.getAllocator())
1164-
AllowInaccessibleMember(cs, baseType, member, name, locator);
1160+
AllowInaccessibleMember *AllowInaccessibleMember::create(
1161+
ConstraintSystem &cs, Type baseType, ValueDecl *member, DeclNameRef name,
1162+
ConstraintLocator *locator, bool isMissingImport) {
1163+
return new (cs.getAllocator()) AllowInaccessibleMember(
1164+
cs, baseType, member, name, locator, isMissingImport);
11651165
}
11661166

11671167
bool AllowAnyObjectKeyPathRoot::diagnose(const Solution &solution,

lib/Sema/CSSimplify.cpp

+38-21
Original file line numberDiff line numberDiff line change
@@ -10454,30 +10454,45 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
1045410454
if (result.ViableCandidates.empty() && result.UnviableCandidates.empty() &&
1045510455
includeInaccessibleMembers) {
1045610456
NameLookupOptions lookupOptions = defaultMemberLookupOptions;
10457-
10458-
// Ignore access control so we get candidates that might have been missed
10459-
// before.
10460-
lookupOptions |= NameLookupFlags::IgnoreAccessControl;
1046110457

10462-
auto lookup =
10463-
TypeChecker::lookupMember(DC, instanceTy, memberName,
10464-
memberLoc, lookupOptions);
10465-
for (auto entry : lookup) {
10466-
auto *cand = entry.getValueDecl();
10458+
// Local function that looks up additional candidates using the given lookup
10459+
// options, recording the results as unviable candidates.
10460+
auto lookupUnviable =
10461+
[&](NameLookupOptions lookupOptions,
10462+
MemberLookupResult::UnviableReason reason) -> bool {
10463+
auto lookup = TypeChecker::lookupMember(DC, instanceTy, memberName,
10464+
memberLoc, lookupOptions);
10465+
for (auto entry : lookup) {
10466+
auto *cand = entry.getValueDecl();
10467+
10468+
// If the result is invalid, skip it.
10469+
if (cand->isInvalid()) {
10470+
result.markErrorAlreadyDiagnosed();
10471+
break;
10472+
}
10473+
10474+
if (excludedDynamicMembers.count(cand))
10475+
continue;
1046710476

10468-
// If the result is invalid, skip it.
10469-
if (cand->isInvalid()) {
10470-
result.markErrorAlreadyDiagnosed();
10471-
return result;
10477+
result.addUnviable(getOverloadChoice(cand, /*isBridged=*/false,
10478+
/*isUnwrappedOptional=*/false),
10479+
reason);
1047210480
}
1047310481

10474-
if (excludedDynamicMembers.count(cand))
10475-
continue;
10482+
return !lookup.empty();
10483+
};
1047610484

10477-
result.addUnviable(getOverloadChoice(cand, /*isBridged=*/false,
10478-
/*isUnwrappedOptional=*/false),
10479-
MemberLookupResult::UR_Inaccessible);
10480-
}
10485+
// Ignore access control so we get candidates that might have been missed
10486+
// before.
10487+
if (lookupUnviable(lookupOptions | NameLookupFlags::IgnoreAccessControl,
10488+
MemberLookupResult::UR_Inaccessible))
10489+
return result;
10490+
10491+
// Ignore missing import statements in order to find more candidates that
10492+
// might have been missed before.
10493+
if (lookupUnviable(lookupOptions | NameLookupFlags::IgnoreMissingImports,
10494+
MemberLookupResult::UR_MissingImport))
10495+
return result;
1048110496
}
1048210497

1048310498
return result;
@@ -10678,9 +10693,11 @@ static ConstraintFix *fixMemberRef(
1067810693
}
1067910694

1068010695
case MemberLookupResult::UR_Inaccessible:
10696+
case MemberLookupResult::UR_MissingImport:
1068110697
assert(choice.isDecl());
10682-
return AllowInaccessibleMember::create(cs, baseTy, choice.getDecl(),
10683-
memberName, locator);
10698+
return AllowInaccessibleMember::create(
10699+
cs, baseTy, choice.getDecl(), memberName, locator,
10700+
*reason == MemberLookupResult::UR_MissingImport);
1068410701

1068510702
case MemberLookupResult::UR_UnavailableInExistential: {
1068610703
return choice.isDecl()

lib/Sema/CodeSynthesis.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -1152,7 +1152,8 @@ static void collectNonOveriddenSuperclassInits(
11521152
superclassDecl->synthesizeSemanticMembersIfNeeded(
11531153
DeclBaseName::createConstructor());
11541154

1155-
NLOptions subOptions = (NL_QualifiedDefault | NL_IgnoreAccessControl);
1155+
NLOptions subOptions =
1156+
(NL_QualifiedDefault | NL_IgnoreAccessControl | NL_IgnoreMissingImports);
11561157
SmallVector<ValueDecl *, 4> lookupResults;
11571158
subclass->lookupQualified(
11581159
superclassDecl, DeclNameRef::createConstructor(),

lib/Sema/PreCheckExpr.cpp

+13-4
Original file line numberDiff line numberDiff line change
@@ -525,10 +525,6 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
525525
const ValueDecl *first = inaccessibleResults.front().getValueDecl();
526526
auto accessLevel =
527527
first->getFormalAccessScope().accessLevelForDiagnostics();
528-
if (accessLevel == AccessLevel::Public &&
529-
diagnoseMissingImportForMember(first, DC, Loc))
530-
return errorResult();
531-
532528
Context.Diags.diagnose(Loc, diag::candidate_inaccessible, first,
533529
accessLevel);
534530

@@ -544,6 +540,19 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
544540
return errorResult();
545541
}
546542

543+
// Try ignoring missing imports.
544+
relookupOptions |= NameLookupFlags::IgnoreMissingImports;
545+
auto nonImportedResults =
546+
TypeChecker::lookupUnqualified(DC, LookupName, Loc, relookupOptions);
547+
if (nonImportedResults) {
548+
const ValueDecl *first = nonImportedResults.front().getValueDecl();
549+
diagnoseMissingImportForMember(first, DC, Loc);
550+
551+
// Don't try to recover here; we'll get more access-related diagnostics
552+
// downstream if the type of the inaccessible decl is also inaccessible.
553+
return errorResult();
554+
}
555+
547556
// TODO: Name will be a compound name if it was written explicitly as
548557
// one, but we should also try to propagate labels into this.
549558
DeclNameLoc nameLoc = UDRE->getNameLoc();

lib/Sema/TypeCheckNameLookup.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,8 @@ convertToUnqualifiedLookupOptions(NameLookupOptions options) {
221221
newOptions |= UnqualifiedLookupFlags::IncludeUsableFromInline;
222222
if (options.contains(NameLookupFlags::ExcludeMacroExpansions))
223223
newOptions |= UnqualifiedLookupFlags::ExcludeMacroExpansions;
224+
if (options.contains(NameLookupFlags::IgnoreMissingImports))
225+
newOptions |= UnqualifiedLookupFlags::IgnoreMissingImports;
224226

225227
return newOptions;
226228
}
@@ -324,6 +326,8 @@ LookupResult TypeChecker::lookupMember(DeclContext *dc,
324326
NLOptions subOptions = (NL_QualifiedDefault | NL_ProtocolMembers);
325327
if (options.contains(NameLookupFlags::IgnoreAccessControl))
326328
subOptions |= NL_IgnoreAccessControl;
329+
if (options.contains(NameLookupFlags::IgnoreMissingImports))
330+
subOptions |= NL_IgnoreMissingImports;
327331

328332
// We handle our own overriding/shadowing filtering.
329333
subOptions &= ~NL_RemoveOverridden;
@@ -420,6 +424,8 @@ LookupTypeResult TypeChecker::lookupMemberType(DeclContext *dc,
420424

421425
if (options.contains(NameLookupFlags::IgnoreAccessControl))
422426
subOptions |= NL_IgnoreAccessControl;
427+
if (options.contains(NameLookupFlags::IgnoreMissingImports))
428+
subOptions |= NL_IgnoreMissingImports;
423429
if (options.contains(NameLookupFlags::IncludeUsableFromInline))
424430
subOptions |= NL_IncludeUsableFromInline;
425431

0 commit comments

Comments
 (0)