Skip to content

[AST] Remove ParameterTypeFlags from ParenType and TupleType #60263

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

Conversation

hamishknight
Copy link
Contributor

The last clients that relied on stashing parameter type flags on these types are now gone.

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Looks good to me, I only have one reservation and it's about ParameterFlagHandling and letting callers get away with dropping flags.

@hamishknight hamishknight force-pushed the 2-toil-2-tuple branch 2 times, most recently from c7b6f00 to dab78c6 Compare August 1, 2022 17:17
@hamishknight
Copy link
Contributor Author

@swift-ci please test

@hamishknight
Copy link
Contributor Author

@swift-ci please test source compatibility

@hamishknight
Copy link
Contributor Author

GitHub got the wrong timestamps for the force push, correct diff is https://github.com/apple/swift/compare/dab78c634d2adfc350f8f5085fce358040a4d2c8..69f59bbc6c8b2201b64b0100a8a62bffb79775e2

Previously we relied on `TupleTypeElt::getType`
returning an `InOutType` to fail the tuple type
matching logic. Instead, add logic to reject any
inout arguments up-front with a more specific
diagnostic.

Also, while we're here, strip the `_const`
parameter flag, as it's not something that needs
to be considered for tuple construction.
Callers may either assert that the parameter flags
are empty, or ask for them to be dropped.
SILGen shouldn't care whether or not a self
parameter is wrapped in a paren with ownership.
The last clients that relied on stashing parameter
type flags on these types are now gone.
@hamishknight hamishknight force-pushed the 2-toil-2-tuple branch 2 times, most recently from 69f59bb to e0d8e4f Compare August 2, 2022 12:57
@hamishknight
Copy link
Contributor Author

@swift-ci please test

@hamishknight
Copy link
Contributor Author

@xedin Noticed that we didn't have logic to specifically reject inout arguments for tuple construction (previously we were relying on TupleTypeElt::getType producing InOutType to fail the match). Added logic to reject inout arguments up-front in 1bf954c, mind taking a look?

@xedin
Copy link
Contributor

xedin commented Aug 2, 2022

@hamishknight Looks great!

@hamishknight
Copy link
Contributor Author

🚢

@hamishknight hamishknight merged commit 8bd4100 into swiftlang:main Aug 2, 2022
@hamishknight hamishknight deleted the 2-toil-2-tuple branch August 2, 2022 19:12
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.

3 participants