Skip to content
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

Updated Bad diagnostic errors #65531

Closed
wants to merge 15 commits into from

Conversation

dk-talks
Copy link

Fixes #64903

I have updated the error messages which were not very clear to the user.

Environment
Swift version 5.9-dev (LLVM 6a04c4848990c70, Swift f2bac1a22939a4d)

This is my first pull request on this project. Please scold me if I have made any mistake.

@dk-talks
Copy link
Author

dk-talks commented May 3, 2023

Hey @LucianoPAlmeida I have committed the new changes you suggested, please have a look

@LucianoPAlmeida
Copy link
Contributor

Sorry, been a bit busy lately with work so I just glanced on the basics when commenting.
Now was trying to get more context I see that the exercise here is to try to make the wording of diagnostic to be clear on what the language rule is and tell that to the user.
Here are a few things to think about:

struct SS<T> : T { } // expected-error{{cannot inherit from non-protocol type 'T'}}

It is still not clear what cannot inherit from not protocol types. So you can start thinking about the rules about inheritance for struct types and how to describe that in a succinct, but clear way.

Same applies to other cases, so you can start with questions such as:
What is the language rules for inheritance of each construct class, struct, enum and extensions of each of those?
And how improve the current diagnostics to express that in a more clear way?

@@ -3056,7 +3056,7 @@ ERROR(duplicate_inheritance,none,
WARNING(duplicate_anyobject_class_inheritance,none,
"redundant inheritance from 'AnyObject' and Swift 3 'class' keyword", ())
ERROR(inheritance_from_protocol_with_superclass,none,
"inheritance from class-constrained protocol composition type %0", (Type))
"cannot inherit from class-constrained protocol composition type %0", (Type))
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 we need to split this into multiple diagnostics and change phrasing. There are a couple of issues here which are not clearly stated by the diagnostic:

  • There are only curtain types allowed in inheritance clause, this could be fixed by splitting diagnostic into multiple ones;
  • Inheritance in extension is even more restricted to protocols only;
  • Message is not very clear about the why to give actionable guidance to the developer how to fix the problem;
    • I think it might be useful to separate type kind + name information into a note and give error more information about what is going on here;

@dk-talks dk-talks closed this May 29, 2023
@dk-talks dk-talks deleted the dk_talks/diagnostic branch May 29, 2023 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad diagnostic when inheriting from class in extension
3 participants