Skip to content

Error message for obsoleted protocol composition syntax should say "types" instead of "protocols" #62518

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

Closed
AnthonyLatsis opened this issue Dec 12, 2022 · 17 comments · Fixed by #63574
Labels
compiler The Swift compiler itself diagnostics QoI Bug: Diagnostics Quality of Implementation good first issue Good for newcomers improvement parser Area → compiler: The legacy C++ parser

Comments

@AnthonyLatsis
Copy link
Collaborator

The following syntax error is emitted when the parser spots obsoleted protocol composition syntax:

typealias C = protocol<P, Q> // error: 'protocol<...>' composition syntax has been removed; join the protocols using '&'

The problem with this message is that "protocols" is not an accurate or prudent description of the components at this compilation stage:

  • Protocol compositions are not limited to protocols.
  • A component can be written in terms of a member type, such as a nested typealias.
  • The parser has no knowledge of whether the components are legal, namely, whether they really reference protocols or classes.

A more neutral "types" is preferable.

@AnthonyLatsis AnthonyLatsis added improvement parser Area → compiler: The legacy C++ parser compiler The Swift compiler itself good first issue Good for newcomers diagnostics QoI Bug: Diagnostics Quality of Implementation labels Dec 12, 2022
@hborla
Copy link
Member

hborla commented Dec 19, 2022

Hmm, I'm not sure that "types" is the right word to use here. The things that are joined together with & in a composition are constraints. Compositions can be used for existential types, or for generic requirements on type parameters. We've been trying to guide people away from thinking of protocols and other constraints as types.

@AnthonyLatsis
Copy link
Collaborator Author

Good point, wording them as constraints (or type constraints per the language guide) sounds better.

@IceCurrent
Copy link

So we change the word "types" to "type constraints"?
Also is it the right way to do it, as I've done ? (because it is failing the smoke test)
@AnthonyLatsis @hborla

@AnthonyLatsis
Copy link
Collaborator Author

So we change the word "types" to "type constraints"?

Sounds good to me. It’s totally fine to do this in the same pull request btw, no need to close it.

Also is it the right way to do it, as I've done ? (because it is failing the smoke test)

Yeah, you simply forgot to update a test somewhere — check out the logs.

Rajveer100 pushed a commit to Rajveer100/swift that referenced this issue Feb 10, 2023
…s' for obsoleted error decomposition syntax error message
@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 10, 2023

Have done the necessary changes @AnthonyLatsis

@AnthonyLatsis
Copy link
Collaborator Author

Have you checked in with @IceCurrent? They might still be interested in wrapping this up.

@Rajveer100
Copy link
Contributor

I didn't check with him, but looks like he closed his pull request around 2 months back and didn't revert, so I just did the changes.

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 10, 2023

Also, I actually by mistake committed and pushed changes of this issue in the same branch of the previous pull request and since the branch is same, it's reflected in the previous issue too...
What's the best I can do to resolve this...

@AnthonyLatsis
Copy link
Collaborator Author

Soft-reset those changes, stash them, then unstash them in a new branch, then force-push the other branch.

@Rajveer100
Copy link
Contributor

Thanks!

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 10, 2023

@AnthonyLatsis I have committed the changes and also did the testing, can I open a pull request?

Rajveer100 pushed a commit to Rajveer100/swift that referenced this issue Feb 10, 2023
@AnthonyLatsis
Copy link
Collaborator Author

I cannot forbid you to, but I will suggest to at least try and reach out to the person that last worked in this. We do not want fixing issues to be a race between contributors (unless it is a race that has been agreed on 😅).

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 10, 2023

Lol!

@Rajveer100
Copy link
Contributor

Rajveer100 commented Feb 10, 2023

Also, I see the commits of my ongoing PR in the new PR, is that normal to happen, because when I researched, it seemed that it happens a lot and it's a Git issue?

@AnthonyLatsis
Copy link
Collaborator Author

AnthonyLatsis commented Feb 10, 2023

That means that your new branch branches off from the branch associated with the ongoing PR. You should set the new branch to track main instead and rebase.

@Rajveer100
Copy link
Contributor

@AnthonyLatsis I have created a pull request, please review it.

@AnthonyLatsis
Copy link
Collaborator Author

@IceCurrent Are you interested in wrapping this up?

Rajveer100 pushed a commit to Rajveer100/swift that referenced this issue Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler The Swift compiler itself diagnostics QoI Bug: Diagnostics Quality of Implementation good first issue Good for newcomers improvement parser Area → compiler: The legacy C++ parser
Projects
None yet
4 participants