Skip to content

redo 'without' operator parsing so it's within parseType to match C++ parser #1588

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

Merged
merged 1 commit into from
Apr 27, 2023

Conversation

kavon
Copy link
Member

@kavon kavon commented Apr 26, 2023

I decided to change the parsing on the C++ side to instead look for suppression in parseType as it yielded a better implementation. As such, "suppressed" inherited entries are now gone and you can now have general suppressed types being parsed.

@kavon kavon force-pushed the better-parsing-suppression branch from 019a02d to 24925c3 Compare April 26, 2023 01:01
@kavon
Copy link
Member Author

kavon commented Apr 26, 2023

@swift-ci please test

@kavon kavon marked this pull request as ready for review April 26, 2023 01:03
@kavon kavon requested a review from ahoppen as a code owner April 26, 2023 01:03
This basically moves the logic out from only when encountering an
inheritance entry and instead when generally parsing a type, since
that was a better way to go on the C++ side.
@kavon kavon force-pushed the better-parsing-suppression branch from 24925c3 to d141df5 Compare April 26, 2023 01:08
@kavon
Copy link
Member Author

kavon commented Apr 26, 2023

@swift-ci please test

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

I am not following the evolution proposal but do we really want to support ~ in any place that supports types. As it stands right now, once we integrate the new parser into the compiler, ASTGen will need to diagnose that ~ cannot be used for every syntactic position that supports types.

@kavon
Copy link
Member Author

kavon commented Apr 26, 2023

Eventually it will need to be supported in where clauses and in parsing generic parameters like <T: ~Copyable>. At the moment TypeCheckType.cpp flags those appearances as illegal, so I think ASTGen can safely just map it over to a SuppressedTyprRepr as in swiftlang/swift#64699

@kavon
Copy link
Member Author

kavon commented Apr 26, 2023

So you can think of this as an improvement in terms of diagnostics, since now we give a good diagnostic complaining about uses of ~Type in other places, as people are bound to try it elsewhere. Without this change they'd get really bizarre parsing errors. Those diagnostics happen during type checking, though, not parsing. See test/Sema/without_implicit_conformance.swift in that PR for examples of the messages.

@kavon
Copy link
Member Author

kavon commented Apr 26, 2023

@swift-ci please test windows

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.

2 participants