Skip to content

Commit 4341ef3

Browse files
committed
Move CalleeCandidateInfo to using NameLookupFlags::IgnoreAccessibility
for initializer lookup, allowing it to produce more specific diagnostics when referring to a private initializer that the compiler can see. In addition to improving diagnostics, this allows us to eliminate the NoPublicInitializers failure kind.
1 parent 302e8cd commit 4341ef3

File tree

6 files changed

+86
-43
lines changed

6 files changed

+86
-43
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ ERROR(candidate_inaccessible,none,
100100
"%0 is inaccessible due to '%select{private|internal|PUBLIC}1' "
101101
"protection level", (DeclName, Accessibility))
102102

103+
ERROR(init_candidate_inaccessible,none,
104+
"%0 initializer is inaccessible due to '%select{private|internal|PUBLIC}1' "
105+
"protection level", (Type, Accessibility))
106+
107+
103108
ERROR(cannot_pass_rvalue_mutating_subelement,none,
104109
"cannot use mutating member on immutable value: %0",
105110
(StringRef))

lib/Sema/CSDiag.cpp

Lines changed: 76 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,6 @@ void Failure::dump(SourceManager *sm, raw_ostream &out) const {
4747
<< " and " << getSecondType().getString();
4848
break;
4949

50-
case NoPublicInitializers:
51-
out << getFirstType().getString()
52-
<< " does not have any public initializers";
53-
break;
54-
5550
case IsNotMaterializable:
5651
out << getFirstType().getString() << " is not materializable";
5752
break;
@@ -593,14 +588,6 @@ static bool diagnoseFailure(ConstraintSystem &cs, Failure &failure,
593588
}
594589
// FIXME: diagnose other cases
595590
return false;
596-
597-
case Failure::NoPublicInitializers: {
598-
tc.diagnose(loc, diag::no_accessible_initializers, failure.getFirstType())
599-
.highlight(range);
600-
if (targetLocator && !useExprLoc)
601-
noteTargetOfDiagnostic(cs, &failure, targetLocator);
602-
break;
603-
}
604591

605592
case Failure::IsNotMaterializable: {
606593
tc.diagnose(loc, diag::cannot_bind_generic_parameter_to_type,
@@ -992,6 +979,7 @@ namespace {
992979
enum CandidateCloseness {
993980
CC_ExactMatch, ///< This is a perfect match for the arguments.
994981
CC_Unavailable, ///< Marked unavailable with @available.
982+
CC_Inaccessible, ///< Not accessible from the current context.
995983
CC_NonLValueInOut, ///< First arg is inout but no lvalue present.
996984
CC_SelfMismatch, ///< Self argument mismatches.
997985
CC_OneArgumentNearMismatch, ///< All arguments except one match, near miss.
@@ -1157,6 +1145,11 @@ namespace {
11571145
/// argument labels don't match up, diagnose that error and return true.
11581146
bool diagnoseAnyStructuralArgumentError(Expr *fnExpr, Expr *argExpr);
11591147

1148+
/// Emit a diagnostic and return true if this is an error condition we can
1149+
/// handle uniformly. This should be called after filtering the candidate
1150+
/// list.
1151+
bool diagnoseSimpleErrors(SourceLoc loc);
1152+
11601153
void dump() const LLVM_ATTRIBUTE_USED;
11611154

11621155
private:
@@ -1199,6 +1192,19 @@ void CalleeCandidateInfo::filterList(ClosenessPredicate predicate) {
11991192
!CS->TC.getLangOpts().DisableAvailabilityChecking)
12001193
declCloseness.first = CC_Unavailable;
12011194

1195+
// Likewise, if the candidate is inaccessible from the scope it is being
1196+
// accessed from, mark it as inaccessible or a general mismatch.
1197+
if (!decl.decl->isAccessibleFrom(CS->DC)) {
1198+
// If this was an exact match, downgrade it to inaccessible, so that
1199+
// accessible decls that are also an exact match will take precedence.
1200+
// Otherwise consider it to be a general mismatch so we only list it in
1201+
// an overload set as a last resort.
1202+
if (declCloseness.first == CC_ExactMatch)
1203+
declCloseness.first = CC_Inaccessible;
1204+
else
1205+
declCloseness.first = CC_GeneralMismatch;
1206+
}
1207+
12021208
closenessList.push_back(declCloseness);
12031209
closeness = std::min(closeness, closenessList.back().first);
12041210
}
@@ -1393,11 +1399,12 @@ void CalleeCandidateInfo::collectCalleeCandidates(Expr *fn) {
13931399

13941400
if (auto TE = dyn_cast<TypeExpr>(fn)) {
13951401
// It's always a metatype type, so use the instance type name.
1396-
auto instanceType =TE->getType()->castTo<MetatypeType>()->getInstanceType();
1402+
auto instanceType =TE->getInstanceType();
13971403

13981404
// TODO: figure out right value for isKnownPrivate
13991405
if (!instanceType->getAs<TupleType>()) {
1400-
auto ctors = CS->TC.lookupConstructors(CS->DC, instanceType);
1406+
auto ctors = CS->TC.lookupConstructors(CS->DC, instanceType,
1407+
NameLookupFlags::IgnoreAccessibility);
14011408
for (auto ctor : ctors)
14021409
if (ctor->hasType())
14031410
candidates.push_back({ ctor, 1 });
@@ -1658,9 +1665,23 @@ suggestPotentialOverloads(SourceLoc loc, bool isResult) {
16581665
/// labels don't match up, diagnose that error and return true.
16591666
bool CalleeCandidateInfo::diagnoseAnyStructuralArgumentError(Expr *fnExpr,
16601667
Expr *argExpr) {
1668+
// If we are invoking a constructor and there are absolutely no candidates,
1669+
// then they must all be private.
1670+
if (auto *MTT = fnExpr->getType()->getAs<MetatypeType>()) {
1671+
if (!MTT->getInstanceType()->is<TupleType>() &&
1672+
(size() == 0 ||
1673+
(size() == 1 && isa<ProtocolDecl>(candidates[0].decl)))) {
1674+
CS->TC.diagnose(fnExpr->getLoc(), diag::no_accessible_initializers,
1675+
MTT->getInstanceType());
1676+
return true;
1677+
}
1678+
}
1679+
1680+
16611681
// TODO: We only handle the situation where there is exactly one candidate
16621682
// here.
1663-
if (size() != 1) return false;
1683+
if (size() != 1)
1684+
return false;
16641685

16651686

16661687
auto args = decomposeArgParamType(argExpr->getType());
@@ -1829,7 +1850,35 @@ bool CalleeCandidateInfo::diagnoseAnyStructuralArgumentError(Expr *fnExpr,
18291850
return false;
18301851
}
18311852

1853+
/// Emit a diagnostic and return true if this is an error condition we can
1854+
/// handle uniformly. This should be called after filtering the candidate
1855+
/// list.
1856+
bool CalleeCandidateInfo::diagnoseSimpleErrors(SourceLoc loc) {
1857+
// Handle symbols marked as explicitly unavailable.
1858+
if (closeness == CC_Unavailable)
1859+
return CS->TC.diagnoseExplicitUnavailability(candidates[0].decl, loc,
1860+
CS->DC);
1861+
1862+
// Handle symbols that are matches, but are not accessible from the current
1863+
// scope.
1864+
if (closeness == CC_Inaccessible) {
1865+
auto decl = candidates[0].decl;
1866+
if (auto *CD = dyn_cast<ConstructorDecl>(decl)) {
1867+
CS->TC.diagnose(loc, diag::init_candidate_inaccessible,
1868+
CD->getResultType(), decl->getFormalAccess());
1869+
1870+
} else {
1871+
CS->TC.diagnose(loc, diag::candidate_inaccessible, decl->getName(),
1872+
decl->getFormalAccess());
1873+
}
1874+
for (auto cand : candidates)
1875+
CS->TC.diagnose(cand.decl, diag::decl_declared_here, decl->getName());
1876+
1877+
return true;
1878+
}
18321879

1880+
return false;
1881+
}
18331882

18341883

18351884

@@ -3604,13 +3653,10 @@ bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
36043653

36053654
return true;
36063655
}
3607-
3608-
if (calleeInfo.closeness == CC_Unavailable) {
3609-
if (CS->TC.diagnoseExplicitUnavailability(calleeInfo[0].decl,
3610-
SE->getLoc(), CS->DC))
3611-
return true;
3612-
return false;
3613-
}
3656+
3657+
// Diagnose some simple and common errors.
3658+
if (calleeInfo.diagnoseSimpleErrors(SE->getLoc()))
3659+
return true;
36143660

36153661
// If the closest matches all mismatch on self, we either have something that
36163662
// cannot be subscripted, or an ambiguity.
@@ -3811,10 +3857,10 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
38113857
}
38123858

38133859

3814-
// Handle uses of unavailable symbols.
3815-
if (calleeInfo.closeness == CC_Unavailable)
3816-
return CS->TC.diagnoseExplicitUnavailability(calleeInfo[0].decl,
3817-
callExpr->getLoc(), CS->DC);
3860+
// Diagnose some simple and common errors.
3861+
if (calleeInfo.diagnoseSimpleErrors(callExpr->getLoc()))
3862+
return true;
3863+
38183864

38193865
// A common error is to apply an operator that only has inout forms (e.g. +=)
38203866
// to non-lvalues (e.g. a local let). Produce a nice diagnostic for this
@@ -4721,8 +4767,9 @@ bool FailureDiagnosis::visitUnresolvedMemberExpr(UnresolvedMemberExpr *E) {
47214767
}
47224768

47234769
case CC_Unavailable:
4724-
if (CS->TC.diagnoseExplicitUnavailability(candidateInfo[0].decl,
4725-
E->getLoc(), CS->DC))
4770+
case CC_Inaccessible:
4771+
// Diagnose some simple and common errors.
4772+
if (candidateInfo.diagnoseSimpleErrors(E->getLoc()))
47264773
return true;
47274774
return false;
47284775

lib/Sema/CSSimplify.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2193,14 +2193,8 @@ ConstraintSystem::simplifyConstructionConstraint(Type valueType,
21932193
if (isa<AbstractFunctionDecl>(DC))
21942194
lookupOptions |= NameLookupFlags::KnownPrivate;
21952195
auto ctors = TC.lookupConstructors(DC, valueType, lookupOptions);
2196-
if (!ctors) {
2197-
// If we are supposed to record failures, do so.
2198-
if (shouldRecordFailures()) {
2199-
recordFailure(locator, Failure::NoPublicInitializers, valueType);
2200-
}
2201-
2196+
if (!ctors)
22022197
return SolutionKind::Error;
2203-
}
22042198

22052199
auto &context = getASTContext();
22062200
auto name = context.Id_init;

lib/Sema/ConstraintSystem.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,6 @@ class Failure : public llvm::FoldingSetNode {
365365
IsNotBridgedToObjectiveC,
366366
/// \brief The type is not allowed to be an l-value.
367367
IsForbiddenLValue,
368-
/// Type has no public initializers.
369-
NoPublicInitializers,
370368
/// The type is not materializable.
371369
IsNotMaterializable,
372370
};
@@ -433,7 +431,6 @@ class Failure : public llvm::FoldingSetNode {
433431
getSecondType());
434432

435433
case IsNotBridgedToObjectiveC:
436-
case NoPublicInitializers:
437434
return Profile(id, locator, kind, resolvedOverloadSets, getFirstType(),
438435
value);
439436
}

test/NameBinding/Inputs/accessibility_other.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ extension Foo {
1111
}
1212

1313
struct PrivateInit {
14-
private init() {}
14+
private init() {} // expected-note {{'init' declared here}}
1515
}
1616

1717
extension Foo {

test/NameBinding/accessibility.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ Foo.a()
5252
Foo.b()
5353
Foo.c() // expected-error {{'c' is inaccessible due to 'private' protection level}}
5454

55-
_ = Foo() // expected-error {{'Foo' cannot be constructed because it has no accessible initializers}}
55+
_ = Foo() // expected-error {{'Foo' initializer is inaccessible due to 'internal' protection level}}
5656
// TESTABLE-NOT: :[[@LINE-1]]:{{[^:]+}}:
57-
PrivateInit() // expected-error {{'PrivateInit' cannot be constructed because it has no accessible initializers}}
58-
// TESTABLE: :[[@LINE-1]]:{{[^:]+}}: error: 'PrivateInit' cannot be constructed because it has no accessible initializers
57+
_ = PrivateInit() // expected-error {{'PrivateInit' initializer is inaccessible due to 'private' protection level}}
58+
// TESTABLE: :[[@LINE-1]]:{{[^:]+}}: error: 'PrivateInit' initializer is inaccessible due to 'private' protection level
5959

6060
var s = StructWithPrivateSetter()
6161
s.x = 42 // expected-error {{cannot assign to property: 'x' setter is inaccessible}}

0 commit comments

Comments
 (0)