Split off ArgumentShuffleExpr from TupleShuffleExpr #23458
Merged
Conversation
This file contains 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
Right now we use TupleShuffleExpr for two completely different things: - Tuple conversions, where elements can be re-ordered and labels can be introduced/eliminated - Complex argument lists, involving default arguments or varargs The first case does not allow default arguments or varargs, and the second case does not allow re-ordering or introduction/elimination of labels. Furthermore, the first case has a representation limitation that prevents us from expressing tuple conversions that change the type of tuple elements. For all these reasons, it is better if we use two separate Expr kinds for these purposes. For now, just make an identical copy of TupleShuffleExpr and call it ArgumentShuffleExpr. In CSApply, use ArgumentShuffleExpr when forming the arguments to a call, and keep using TupleShuffleExpr for tuple conversions. Each usage of TupleShuffleExpr has been audited to see if it should instead look at ArgumentShuffleExpr. In sequent commits I plan on redesigning TupleShuffleExpr to correctly represent all tuple conversions without any unnecessary baggage. Longer term, we actually want to change the representation of CallExpr to directly store an argument list; then instead of a single child expression that must be a ParenExpr, TupleExpr or ArgumentShuffleExpr, all CallExprs will have a uniform representation and ArgumentShuffleExpr will go away altogether. This should reduce memory usage and radically simplify parts of SILGen.
Before extending TupleShuffleExpr to represent all tuple conversions allowed by the constraint solver, remove the parts of TupleShuffleExpr that are no longer needed; this is support for default arguments, varargs, and scalar-to-tuple and tuple-to-scalar conversions.
slavapestov
force-pushed
the
tuple-conversions-part-1
branch
from
548613e to
428c709
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
|
@xedin This may interest you as well. |
Azoy
reviewed
Mar 22, 2019
| printRec(E->getSubExpr()); | ||
| PrintWithColorRAII(OS, ParenthesisColor) << ')'; | ||
| } | ||
| void visitArgumentShuffleExpr(ArgumentShuffleExpr *E) { | ||
| printCommon(E, "tuple_shuffle_expr"); |
There was a problem hiding this comment.
Did you mean to also update this to argument_shuffle_expr?
There was a problem hiding this comment.
Good catch. I grepped for TupleShuffleExpr but forgot to check other spellings. I'll fix this in a follow-up PR.
This was referenced Mar 26, 2019
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.
Right now we use TupleShuffleExpr for two completely different things:
introduced/eliminated
The first case does not allow default arguments or varargs, and the
second case does not allow re-ordering or introduction/elimination
of labels. Furthermore, the first case has a representation limitation
that prevents us from expressing tuple conversions that change the
type of tuple elements.
For all these reasons, it is better if we use two separate Expr kinds
for these purposes.
In sequent commits I plan on redesigning TupleShuffleExpr to correctly
represent all tuple conversions without any unnecessary baggage.
Longer term, we actually want to change the representation of CallExpr
to directly store an argument list; then instead of a single child
expression that must be a ParenExpr, TupleExpr or ArgumentShuffleExpr,
all CallExprs will have a uniform representation and ArgumentShuffleExpr
will go away altogether. This should reduce memory usage and radically
simplify parts of SILGen.
(Progress on rdar://12340004, https://bugs.swift.org/browse/SR-2672.)