Improve how we work with pack-expansion abstraction patterns in SILGen #64420
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
opened generic environments Finding these is very hot for these environments, so doing it once is a pretty nice win in both speed and code complexity. I'm not actually using this yet.
First, we need this to work on both lowered and unlowered types, so Type::subst is problematic: it'll assert if it sees a type like SILFunctionType. In this case, the substitution is simple enough that that's never a problem, but Type::subst doesn't know that, and the assertion is generally a good one. Second, we need this to not recurse into nested pack expansions. Third, we need this to not mess around with any existing element archetypes we might see in the type, so mapping in and out of context is not really okay. Fortunately, because we're mapping between structures (pack and element archetypes) that are guaranteed to have the same constraints, this transformation is really easy and we can just do it with transformRec.
As I've been iterating on this work, I've been gradually mulling these over, and I think this is the way to go for now. These should make it a lot less cumbersome to write these kinds of traversals correctly. The intent is to the sunset the existing expanded-components stuff after I do a similar pass for function parameters.
|
@swift-ci Please smoke test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Before I get too much deeper into variadic reabstraction, I wanted to take the opportunity to rework the APIs I've been using for doing these walks over abstraction patterns with expansions, because I'm about to use them a lot more. This centralizes the basic logic for doing parallel walking over expanded tuples and function parameter lists and makes the resulting code in different subsystems at least a little bit clearer. It's definitely more successful at improving the tuple code than it is for calls, though:
selfreally makes things complicated, and the argument code tends to gather argument types from a lot of disparate places rather than passing in substituted types.This also includes fixes to
mapPackTypeIntoElementContextandmapContextualPackTypeIntoElementContext.