Skip to content

[AST] #SR-13906 (Improvement) Error Messages #35238

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 2 commits into from
Closed

[AST] #SR-13906 (Improvement) Error Messages #35238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b2007cf
[AST] #SR-13906 (Improvement)
Dec 29, 2020
aad3bc6
Updated Tests for Diagnostics
Jan 18, 2021
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
9 changes: 7 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2633,9 +2633,9 @@ WARNING(anyobject_class_inheritance_deprecated,none,
ERROR(multiple_inheritance,none,
"multiple inheritance from classes %0 and %1", (Type, Type))
ERROR(inheritance_from_non_protocol_or_class,none,
"inheritance from non-protocol, non-class type %0", (Type))
"%0 type cannot inherit from non-protocol, non-class type %1", (DescriptiveDeclKind, Type))
ERROR(inheritance_from_non_protocol,none,
"inheritance from non-protocol type %0", (Type))
"%0 type cannot inherit from non-protocol type %1", (DescriptiveDeclKind, Type))
ERROR(superclass_not_first,none,
"superclass %0 must appear first in the inheritance clause", (Type))
ERROR(superclass_not_open,none,
Expand All @@ -2662,6 +2662,11 @@ ERROR(inheritance_from_cf_class,none,
ERROR(inheritance_from_objc_runtime_visible_class,none,
"cannot inherit from class %0 because it is only visible via the "
"Objective-C runtime", (Identifier))
ERROR(composition_of_protocol_and_anyobject,none,
Copy link
Contributor

Choose a reason for hiding this comment

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

I see there is this tailored diagnostic for composition but not a regression test case for exercise it. We should add the respective test :)

"non-protocol type %0 cannot conform to composition type %1; "
"%0 must be a protocol type",
(Identifier, Type))


// Enums
ERROR(enum_case_access,none,
Expand Down
18 changes: 14 additions & 4 deletions lib/Sema/TypeCheckDeclPrimary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,19 @@ static void checkInheritanceClause(
// AnyObject is not allowed except on protocols.
if (layout.hasExplicitAnyObject &&
!isa<ProtocolDecl>(decl)) {
decl->diagnose(canHaveSuperclass
? diag::inheritance_from_non_protocol_or_class
: diag::inheritance_from_non_protocol,
inheritedTy);
// Check if the inherited type is exclusively AnyObject
// If not, we can be sure of the composition of Protocol(s) with
// AnyObject
if (!layout.isAnyObject()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's more important to break up the diagnostic in the else by making it more specific and using less compiler-jargon-ified language.

Copy link
Author

Choose a reason for hiding this comment

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

You mean breaking down the ternary operator to if-else for canHaveSuperClass?

Copy link
Contributor

Choose a reason for hiding this comment

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

The intent of the original bug was to fix this particular ternary by providing a more specific message. Its branches are the cause of the confusion here.

decl->diagnose(diag::composition_of_protocol_and_anyobject,
typeDecl->getName(), inheritedTy);
} else {
decl->diagnose(canHaveSuperclass
Copy link
Contributor

@augusto2112 augusto2112 Jan 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think the comment left by Codafi, of addressing the else block still needs to be done.

? diag::inheritance_from_non_protocol_or_class
: diag::inheritance_from_non_protocol,
decl->getDescriptiveKind(), inheritedTy);
}

continue;
}

Expand Down Expand Up @@ -342,6 +351,7 @@ static void checkInheritanceClause(
decl->diagnose(canHaveSuperclass
? diag::inheritance_from_non_protocol_or_class
: diag::inheritance_from_non_protocol,
decl->getDescriptiveKind(),
inheritedTy);
// FIXME: Note pointing to the declaration 'inheritedTy' references?
}
Expand Down
2 changes: 1 addition & 1 deletion test/Generics/inheritance.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func testGenericInherit() {
}


struct SS<T> : T { } // expected-error{{inheritance from non-protocol type 'T'}}
struct SS<T> : T { } // expected-error{{generic struct type cannot inherit from non-protocol type 'T'giy }}
Copy link
Contributor

Choose a reason for hiding this comment

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

giy seems like a typo here :)

enum SE<T> : T { case X } // expected-error{{raw type 'T' is not expressible by a string, integer, or floating-point literal}} // expected-error {{SE<T>' declares raw type 'T', but does not conform to RawRepresentable and conformance could not be synthesized}} expected-error{{RawRepresentable conformance cannot be synthesized because raw type 'T' is not Equatable}}

// Also need Equatable for init?(RawValue)
Expand Down
2 changes: 1 addition & 1 deletion test/Parse/diagnostic_missing_func_keyword.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class C1 {
}

class C2 {
class classProperty: Int { 0 } // expected-error {{inheritance from non-protocol, non-class type 'Int'}}
class classProperty: Int { 0 } // expected-error {{class type cannot inherit from non-protocol, non-class type 'Int'}}
// expected-note @-1 {{in declaration of 'classProperty'}}
// expected-error @-2 {{expected declaration}}
}
6 changes: 3 additions & 3 deletions test/decl/inherit/inherit.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ class D3 : Any, A { } // expected-error{{superclass 'A' must appear first in the
class D4 : P & P1, A { } // expected-error{{superclass 'A' must appear first in the inheritance clause}}{{18-21=}}{{12-12=A, }}

// Struct inheriting a class
struct S : A { } // expected-error{{inheritance from non-protocol type 'A'}}
struct S : A { } // expected-error{{struct type cannot inherit from non-protocol type 'A'}}

// Protocol inheriting a class
protocol Q : A { }

// Extension inheriting a class
extension C : A { } // expected-error{{inheritance from non-protocol type 'A'}}
extension C : A { } // expected-error{{extension type cannot inherit from non-protocol type 'A'}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that there is no such term as extension type, to me this should be the same diagnostic as class C: A {} class type cannot inherit ...


// Keywords in inheritance clauses
struct S2 : struct { } // expected-error{{expected type}}
Expand All @@ -56,7 +56,7 @@ class GenericBase<T> {}

class GenericSub<T> : GenericBase<T> {} // okay

class InheritGenericParam<T> : T {} // expected-error {{inheritance from non-protocol, non-class type 'T'}}
class InheritGenericParam<T> : T {} // expected-error {{generic class type cannot inherit from non-protocol, non-class type 'T'}}
class InheritBody : T { // expected-error {{cannot find type 'T' in scope}}
typealias T = A
}
Expand Down
16 changes: 8 additions & 8 deletions test/decl/protocol/conforms/placement.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,20 +105,20 @@ extension ExplicitSub1 : P1 { } // expected-error{{redundant conformance of 'Exp
// ---------------------------------------------------------------------------
// Suppression of synthesized conformances
// ---------------------------------------------------------------------------
class SynthesizedClass1 : AnyObject { } // expected-error{{inheritance from non-protocol, non-class type 'AnyObject'}}
class SynthesizedClass1 : AnyObject { } // expected-error{{class type cannot inherit from non-protocol, non-class type 'AnyObject'}}

class SynthesizedClass2 { }
extension SynthesizedClass2 : AnyObject { } // expected-error{{inheritance from non-protocol type 'AnyObject'}}
extension SynthesizedClass2 : AnyObject { } // expected-error{{extension type cannot inherit from non-protocol type 'AnyObject'}}

class SynthesizedClass3 : AnyObjectRefinement { }

class SynthesizedClass4 { }
extension SynthesizedClass4 : AnyObjectRefinement { }

class SynthesizedSubClass1 : SynthesizedClass1, AnyObject { } // expected-error{{inheritance from non-protocol, non-class type 'AnyObject'}}
class SynthesizedSubClass1 : SynthesizedClass1, AnyObject { } // expected-error{{class type cannot inherit from non-protocol, non-class type 'AnyObject'}}

class SynthesizedSubClass2 : SynthesizedClass2 { }
extension SynthesizedSubClass2 : AnyObject { } // expected-error{{inheritance from non-protocol type 'AnyObject'}}
extension SynthesizedSubClass2 : AnyObject { } // expected-error{{extension type cannot inherit from non-protocol type 'AnyObject'}}

class SynthesizedSubClass3 : SynthesizedClass1, AnyObjectRefinement { }

Expand Down Expand Up @@ -169,12 +169,12 @@ extension MFExplicitSub1 : P1 { } // expected-error{{redundant conformance of 'M
// ---------------------------------------------------------------------------
class MFSynthesizedClass1 { }

extension MFSynthesizedClass2 : AnyObject { } // expected-error{{inheritance from non-protocol type 'AnyObject'}}
extension MFSynthesizedClass2 : AnyObject { } // expected-error{{extension type cannot inherit from non-protocol type 'AnyObject'}}

class MFSynthesizedClass4 { }
extension MFSynthesizedClass4 : AnyObjectRefinement { }

extension MFSynthesizedSubClass2 : AnyObject { } // expected-error{{inheritance from non-protocol type 'AnyObject'}}
extension MFSynthesizedSubClass2 : AnyObject { } // expected-error{{extension type cannot inherit from non-protocol type 'AnyObject'}}

extension MFSynthesizedSubClass3 : AnyObjectRefinement { }

Expand All @@ -198,7 +198,7 @@ extension MMSuper1 : MMP1 { } // expected-warning{{conformance of 'MMSuper1' to
extension MMSuper1 : MMP2a { } // expected-warning{{conformance of 'MMSuper1' to protocol 'MMP2a' was already stated in the type's module 'placement_module_A'}}
extension MMSuper1 : MMP3b { } // okay

extension MMSub1 : AnyObject { } // expected-error{{inheritance from non-protocol type 'AnyObject'}}
extension MMSub1 : AnyObject { } // expected-error{{extension type cannot inherit from non-protocol type 'AnyObject'}}

extension MMSub2 : MMP1 { } // expected-warning{{conformance of 'MMSub2' to protocol 'MMP1' was already stated in the type's module 'placement_module_A'}}
extension MMSub2 : MMP2a { } // expected-warning{{conformance of 'MMSub2' to protocol 'MMP2a' was already stated in the type's module 'placement_module_A'}}
Expand All @@ -209,7 +209,7 @@ extension MMSub2 : MMAnyObjectRefinement { } // okay
extension MMSub3 : MMP1 { } // expected-warning{{conformance of 'MMSub3' to protocol 'MMP1' was already stated in the type's module 'placement_module_A'}}
extension MMSub3 : MMP2a { } // expected-warning{{conformance of 'MMSub3' to protocol 'MMP2a' was already stated in the type's module 'placement_module_A'}}
extension MMSub3 : MMP3b { } // okay
extension MMSub3 : AnyObject { } // expected-error{{inheritance from non-protocol type 'AnyObject'}}
extension MMSub3 : AnyObject { } // expected-error{{extension type cannot inherit from non-protocol type 'AnyObject'}}

extension MMSub4 : MMP1 { } // expected-warning{{conformance of 'MMSub4' to protocol 'MMP1' was already stated in the type's module 'placement_module_B'}}
extension MMSub4 : MMP2a { } // expected-warning{{conformance of 'MMSub4' to protocol 'MMP2a' was already stated in the type's module 'placement_module_B'}}
Expand Down
2 changes: 1 addition & 1 deletion test/decl/protocol/protocol_with_superclass_objc.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
class Base {}

@objc protocol Protocol1 : Base {}
// expected-error@-1 {{inheritance from non-protocol type 'Base'}}
// expected-error@-1 {{protocol type cannot inherit from non-protocol type 'Base'}}

@objc protocol OtherProtocol {}

Expand Down
2 changes: 1 addition & 1 deletion test/decl/protocol/protocols.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func test1() {
}

protocol Bogus : Int {}
// expected-error@-1{{inheritance from non-protocol, non-class type 'Int'}}
// expected-error@-1{{protocol type cannot inherit from non-protocol, non-class type 'Int'}}
// expected-error@-2{{type 'Self' constrained to non-protocol, non-class type 'Int'}}

// Explicit conformance checks (successful).
Expand Down