Add tuple unpacking syntax#23368
Conversation
d4fbee2 to
f9a0430
Compare
dkorpel
left a comment
dkorpel
left a comment
There was a problem hiding this comment.
Glad to see this progress!
but it will require updating error messages in some of the tests
Have you tried running ./run.d fail_compilation AUTO_UPDATE=1 to see how much changes?
| if (e && de.type && de.type.toBasetype().ty == Tvoid) | ||
| e.Ety = TYvoid; |
There was a problem hiding this comment.
This fixes an ICE that happened in the backend when tuple unpacking operated on types with copy and move constructors.
See: tgehr#18
| return fail(); | ||
| } | ||
|
|
||
| import dmd.expressionsem; |
There was a problem hiding this comment.
This file is not supposed to import semantic. I'm fine with merging it like this and refactoring later though.
There was a problem hiding this comment.
IIRC some imports became necessary during rebase because a function we had already been using was moved during upstream refactorings.
|
Not sure why FreeBSD tests are suddenly failing. |
Not sure why either, but it seems to have started with #23374, which introduced that file. |
…#7) * Fix parsing tuple notation without `-preview=tuples` After #5, without `-preview=tuples`, the parser would cause an assert error when finding tokens that satisfy `isTupleNotation`. * Fix parsing malformed auto declaration starting with `(` Change preview check to an assert after last commit. Handle a tuple declaration that doesn't have `=` after the last `)`. The `peekPastParen(&token).value == TOK.assign` check is not needed because the `parseInitializer` parameter is true so `check(TOK.assign)` does the job and provides a good error message if missing. Add test for malformed auto and non-auto tuple declarations. * Remove unnecessary preview check
Add error for `out` parameters. Ignore `auto ref` after parameter error to avoid unpack error.
Avoid lowering to `auto Type x =`, which causes a redundant `auto` error.
This uses the opening `(` position rather than the first component's.
Require semi-colon after TuplePattern = Initializer.
Although we could allow a list of multiple tuple pattern/init pairs,
let's simplify the grammar to:
```
VarDeclarations:
...
TuplePattern = Initializer;
TuplePattern:
`(` TuplePatternComponents `)`
```
Note AutoDeclaration will still support commas, e.g. `auto (x,) = tup, i
= 1;`.
Grammar changes since approved DIP: AutoDeclaration allows interleaved tuple and non-tuple declarators. VarDeclarations now only takes one TuplePattern. Also change Initializer to AssignExpression (`= void` is not allowed).
|
@tgehr maybe I'm misremembering, wouldn't editions mean no more preview/revert flags? In the compiler, the gate would be something like FeatureState tupleUnpackingSyntax()
=> hasEdition(v2026) ? FeatureState.enabled : FeatureState.disabled;See dscope.d for examples. |
You are. Preview flags come first, then things get promoted to an edition or turned on by default, idk about revert that hasn't come up. |
|
Indeed, I think ideally this will go into the default edition within one or two releases. It is not intended to break any existing code, though my understanding is that it does makes sense, as a rule, to be somewhat cautious as this is a completely new feature that does come with somewhat nontrivial parser changes. However, as one data point, OpenD has pulled tuple unpacking from my branch almost two years ago, in September 2024, and no problems of this kind have been reported since then. (It avoided the ICE related to move constructors because it did not pull move constructors, though that would not have been a regression.) |
|
Are we good to merge? |
|
Oh yeah FreeBSD 13.2 CI runner is gone, needs a rebase? EDIT: branch protection needed to be removed for the dead CI runner. It is now removed. |
|
Awesome. Are there follow-up plans to add Python-style syntax for specifying tuples? Both as run-time values and compile-time AliasSeq values? |
9 participants
Implements DIP 1053, "Tuple Unpacking Syntax".
https://github.com/dlang/DIPs/blob/master/DIPs/accepted/DIP1053.md
Many thanks to @MetaLang and @ntrel for their essential help with pushing this over the finish line.
The PR includes a changelog entry and a spec update.
Note that the features in this PR are still gated behind the
-preview=tuplesflag. This is because of:tgehr#18 (comment) (comment by @ntrel )
I.e., enabling the parser changes causes error messages in some of the existing
fail_compilationtests to change. If preferred, I think we can get the test suite to pass even when enabling unpacking syntax unconditionally, but it will require updating error messages in some of the tests. A more conservative approach would be to release with the flag first, to avoid disruption in case there is an undetected bug and it unexpectedly does break something in the wild. As a middle ground, we could also release with the flag on by default but still support-revert=tuples. (Which would still require updating the tests.)I think assuming that the intention is to support either
-preview=tuplesor-revert=tuplesin any release, this PR can be merged as-is, otherwise it might make sense to remove the flag first. (Though I would be happy to finally see this upstreamed in either case.)