[3.0] Fix @escaping printing (from #4905) #4931
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now that TupleTypeElts are simpler in Swift 3 (though they're about to become more complicated for other reasons), most of the cases where we are explicitly constructing ones are really just plain copies or can otherwise use existing helper functions. NFC
Long term, we want to refactor the AST to reflect the current programming model in Swift. This would include refactoring FunctionType to take a list of ParameterTypeElt, or something with a better name, that can contain both the type and flags/bits that are only specific to types in parameter position, such as @autoclosure and @escaping. At the same time, noescape-by-default has severely hurt our ability to print types without significant context, as we either have to choose to too aggressively print @escaping or not print it in every situation it occurs, or both. As a gentle step towards the final solution, without uprooting our overall AST structure, and as a way towards fixing the @escaping printing ails, put these bits on the TupleTypeElt and ParenType, which will serve as a model for what ParameterTypeElt will be like in the future. Re-use these flags on CallArgParam, to leverage shared knowledge in the type system. It is a little painful to tack onto these types, but it's minor and will be overhauled soon, which will eventually result in size savings and less complexity overall. This includes all the constraint system adjustments to make these types work and influence type equality and overload resolution as desired. They are encoded in the module format. Additional tests added.
Switch printing off of using Function's ExtInfo for autoclosure and escaping, and onto the ParameterTypeFlags, which let us do precise and accurate context-sensitive printing of these parameter type attributes. This fixes a huge list of issues where we were printing @escaping for things like optional ObjC completion handlers, among many others. We now correctly print @escaping in more places, and don't print it when it's not correct. Also updates the dumper to be consistent and give a good view of the AST as represented in memory. Tests updated, more involved testing coming soon.
|
@swift-ci please test OS X Platform |
|
@jrose-apple |
|
@DougGregor blessed this for inclusion |
MaxDesiatov
pushed a commit
that referenced
this pull request
Sep 7, 2023
[pull] swiftwasm from main
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
3.0 version of #4905
• Explanation: The ASTPrinter will often print @escaping when it shouldn’t, and omit it when it needs to print it. This tweaks the AST, specifically ParenType and TupleTypeElt to represent the parameter-type-position flags, and the printer can print without requiring undue magic.
• Scope of Issue: This comes up all the time in the generated interfaces when interoperating with ObjC optional completion handlers. It is massively confusing to developers to see the attributes in positions they cannot physically type themselves, and see error messages telling them it’s about the @escaping when it’s really about other things. Furthermore, we often don’t have @escaping in diagnostics when it’s relevant. When developers get bit by this, it’s massively frustrating for them.
• Origination: GM of 3.0, when most developers first saw the most recent noescape-by-default implementation
• Risk: Moderate, I fiddle with the bits of the AST nodes, though only in ways that should be relevant to the ASTPrinter. Otherwise, only the printing of the AST is influenced, not the language or behavior. But, I do have to touch several files in the constraint system to properly propagate these flags.
• Reviewed By: Jordan and Doug
• Testing: Added lit tests
• Directions for QA: none
rdar://problem/28372774