Skip to content

[Diagnostics] Improve diagnostics for invalid type access on existential types. #60671

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 4 commits into from
Aug 22, 2022
Merged

[Diagnostics] Improve diagnostics for invalid type access on existential types. #60671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
268f6e2
[Diagnostics] Improve the diagnostics for invalid associatedtype or
hborla Aug 20, 2022
6d1b8d4
[TypeResolution] Diagnose invalid dependent member types of existential
hborla Aug 20, 2022
ddb5c13
[Diagnostics] Improve the diagnostics for invalid type access on an
hborla Aug 20, 2022
cd7bc86
[Test] Update error message in validation test.
hborla Aug 22, 2022
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
10 changes: 6 additions & 4 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2465,11 +2465,13 @@ WARNING(append_interpolation_access_control,none,

// Protocols and existentials
ERROR(assoc_type_outside_of_protocol,none,
"associated type %0 can only be used with a concrete type or "
"generic parameter base", (DeclNameRef))
"cannot access associated type %0 from %1; use a concrete type or "
"generic parameter base instead",
(DeclNameRef, Type))
ERROR(typealias_outside_of_protocol,none,
"type alias %0 can only be used with a concrete type or "
"generic parameter base", (DeclNameRef))
"cannot access type alias %0 from %1; use a concrete type or "
"generic parameter base instead",
(DeclNameRef, Type))

ERROR(objc_protocol_inherits_non_objc_protocol,none,
"@objc protocol %0 cannot refine non-@objc protocol %1", (Type, Type))
Expand Down
7 changes: 4 additions & 3 deletions lib/Sema/CSDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4297,7 +4297,8 @@ bool AllowTypeOrInstanceMemberFailure::diagnoseAsError() {
}
}

if (BaseType->is<AnyMetatypeType>() && !Member->isStatic()) {
bool isStaticOrTypeMember = Member->isStatic() || isa<TypeDecl>(Member);
if (BaseType->is<AnyMetatypeType>() && !isStaticOrTypeMember) {
auto instanceTy = BaseType;

if (auto *AMT = instanceTy->getAs<AnyMetatypeType>()) {
Expand Down Expand Up @@ -4405,10 +4406,10 @@ bool AllowTypeOrInstanceMemberFailure::diagnoseAsError() {
// static members doesn't make a whole lot of sense
if (isa<TypeAliasDecl>(Member)) {
Diag.emplace(
emitDiagnostic(diag::typealias_outside_of_protocol, Name));
emitDiagnostic(diag::typealias_outside_of_protocol, Name, instanceTy));
} else if (isa<AssociatedTypeDecl>(Member)) {
Diag.emplace(
emitDiagnostic(diag::assoc_type_outside_of_protocol, Name));
emitDiagnostic(diag::assoc_type_outside_of_protocol, Name, instanceTy));
} else if (isa<ConstructorDecl>(Member)) {
Diag.emplace(
emitDiagnostic(diag::construct_protocol_by_name, instanceTy));
Expand Down
26 changes: 22 additions & 4 deletions lib/Sema/TypeCheckType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ Type TypeResolution::resolveDependentMemberType(
// FIXME(ModQual): Reject qualified names immediately; they cannot be
// dependent member types.
Identifier refIdentifier = ref->getNameRef().getBaseIdentifier();
ASTContext &ctx = DC->getASTContext();

switch (stage) {
case TypeResolutionStage::Structural:
Expand All @@ -146,8 +147,6 @@ Type TypeResolution::resolveDependentMemberType(
// Record the type we found.
ref->setValue(nestedType, nullptr);
} else {
ASTContext &ctx = DC->getASTContext();

// Resolve the base to a potential archetype.
// Perform typo correction.
TypoCorrectionResults corrections(ref->getNameRef(), ref->getNameLoc());
Expand Down Expand Up @@ -188,6 +187,25 @@ Type TypeResolution::resolveDependentMemberType(

auto *concrete = ref->getBoundDecl();

if (auto concreteBase = genericSig->getConcreteType(baseTy)) {
bool hasUnboundOpener = !!getUnboundTypeOpener();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be !?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, I want the value of getUnboundTypeOpener as a bool. This !! pattern is used elsewhere in TypeCheckType.cpp on getUnboundTypeOpener to do the same thing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh interesting! I didn’t know we had a !!.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't either until I looked at the other call-sites of TypeChecker::isUnsupportedMemberTypeAccess 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn’t know we had a !!

Some time ago this pattern made me hang for a second too. It's just a double negation and equivalent to (bool)getUnboundTypeOpener() in this context.

switch (TypeChecker::isUnsupportedMemberTypeAccess(concreteBase, concrete,
hasUnboundOpener)) {
case TypeChecker::UnsupportedMemberTypeAccessKind::TypeAliasOfExistential:
ctx.Diags.diagnose(ref->getNameLoc(),
diag::typealias_outside_of_protocol,
ref->getNameRef(), concreteBase);
break;
case TypeChecker::UnsupportedMemberTypeAccessKind::AssociatedTypeOfExistential:
ctx.Diags.diagnose(ref->getNameLoc(),
diag::assoc_type_outside_of_protocol,
ref->getNameRef(), concreteBase);
break;
default:
break;
};
}

// If the nested type has been resolved to an associated type, use it.
if (auto assocType = dyn_cast<AssociatedTypeDecl>(concrete)) {
return DependentMemberType::get(baseTy, assocType);
Expand Down Expand Up @@ -1656,12 +1674,12 @@ static Type resolveNestedIdentTypeComponent(TypeResolution resolution,

case TypeChecker::UnsupportedMemberTypeAccessKind::TypeAliasOfExistential:
diags.diagnose(comp->getNameLoc(), diag::typealias_outside_of_protocol,
comp->getNameRef());
comp->getNameRef(), parentTy);
return ErrorType::get(ctx);

case TypeChecker::UnsupportedMemberTypeAccessKind::AssociatedTypeOfExistential:
diags.diagnose(comp->getNameLoc(), diag::assoc_type_outside_of_protocol,
comp->getNameRef());
comp->getNameRef(), parentTy);
return ErrorType::get(ctx);
}

Expand Down
6 changes: 3 additions & 3 deletions test/decl/typealias/associated_types.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ protocol BaseProto {
associatedtype AssocTy
}
var a: BaseProto.AssocTy = 4
// expected-error@-1{{associated type 'AssocTy' can only be used with a concrete type or generic parameter base}}
// expected-error@-1{{cannot access associated type 'AssocTy' from 'BaseProto'; use a concrete type or generic parameter base instead}}

var a = BaseProto.AssocTy.self
// expected-error@-1{{associated type 'AssocTy' can only be used with a concrete type or generic parameter base}}
// expected-error@-1{{cannot access associated type 'AssocTy' from 'BaseProto'; use a concrete type or generic parameter base instead}}

protocol DerivedProto : BaseProto {
func associated() -> AssocTy // no-warning

func existential() -> BaseProto.AssocTy
// expected-error@-1{{associated type 'AssocTy' can only be used with a concrete type or generic parameter base}}
// expected-error@-1{{cannot access associated type 'AssocTy' from 'BaseProto'; use a concrete type or generic parameter base instead}}
}


Expand Down
19 changes: 16 additions & 3 deletions test/decl/typealias/protocol.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ struct T5 : P5 {
var a: P5.T1 // OK

// Invalid -- cannot represent associated type of existential
var v2: P5.T2 // expected-error {{type alias 'T2' can only be used with a concrete type or generic parameter base}}
var v3: P5.X // expected-error {{type alias 'X' can only be used with a concrete type or generic parameter base}}
var v2: P5.T2 // expected-error {{cannot access type alias 'T2' from 'P5'; use a concrete type or generic parameter base instead}}
var v3: P5.X // expected-error {{cannot access type alias 'X' from 'P5'; use a concrete type or generic parameter base instead}}

// Unqualified reference to typealias from a protocol conformance
var v4: T1 // OK
Expand All @@ -188,7 +188,20 @@ struct T5 : P5 {
var v7: T5.T2 // OK

var v8 = P6.A.self
var v9 = P6.B.self // expected-error {{type alias 'B' can only be used with a concrete type or generic parameter base}}
var v9 = P6.B.self // expected-error {{cannot access type alias 'B' from 'P6'; use a concrete type or generic parameter base instead}}

var v10 = (any P6).A.self
var v11 = (any P6).B.self // expected-error {{cannot access type alias 'B' from 'any P6'; use a concrete type or generic parameter base instead}}

struct Generic<T> {
func okay(value: T.A) where T == any P6 {}

func invalid1(value: T.B) where T == any P6 {}
// expected-error@-1 {{cannot access type alias 'B' from 'any P6'; use a concrete type or generic parameter base instead}}

func invalid2(value: T.A) where T == any P5 {}
// expected-error@-1 {{cannot access associated type 'A' from 'any P5'; use a concrete type or generic parameter base instead}}
}
}

// Unqualified lookup finds typealiases in protocol extensions
Expand Down
2 changes: 1 addition & 1 deletion test/type/subclass_composition.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func dependentMemberTypes<T : BaseIntAndP2>(
_: T.FullyConcrete,

_: BaseIntAndP2.DependentInConcreteConformance, // FIXME expected-error {{}}
_: BaseIntAndP2.DependentProtocol, // expected-error {{type alias 'DependentProtocol' can only be used with a concrete type or generic parameter base}}
_: BaseIntAndP2.DependentProtocol, // expected-error {{cannot access type alias 'DependentProtocol' from 'BaseIntAndP2' (aka 'Base<Int> & P2'); use a concrete type or generic parameter base instead}}
_: BaseIntAndP2.DependentClass,
_: BaseIntAndP2.FullyConcrete) {}

Expand Down
2 changes: 1 addition & 1 deletion validation-test/compiler_crashers_2_fixed/0159-rdar40009245.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ protocol P {
associatedtype A : P where A.X == Self
// expected-error@-1{{'X' is not a member type of type 'Self.A'}}
associatedtype X : P where P.A == Self
// expected-error@-1{{associated type 'A' can only be used with a concrete type or generic parameter base}}
// expected-error@-1{{cannot access associated type 'A' from 'P'; use a concrete type or generic parameter base instead}}
}