Skip to content
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

PEP 646: Explain the results of the new grammar/compiler change #2189

Merged
merged 2 commits into from
Dec 13, 2021
Merged

PEP 646: Explain the results of the new grammar/compiler change #2189

merged 2 commits into from
Dec 13, 2021

Conversation

mrahtz
Copy link
Contributor

@mrahtz mrahtz commented Dec 10, 2021

@pradeep90 @gvanrossum @stroxler

I'm still not entirely sure that having the compiler emit the equivalent of next(iter(Ts)) for *args: *Ts is the best solution. The alternatives I can think of are:

  1. [*Ts][0] (uglier, but the bytecode ends up being a little simpler)
  2. Directly accessing the property Ts._unpacked (less ugly, but less consistent with other uses of the star operator)

Having said that, I also think next(iter(Ts)) is Good Enough (tm) - and in the first place (I think?) it's not too critical an issue because it's an implementation detail. So happy to discuss this, but also happy not to discuss this :)

@TeamSpen210
Copy link

Looking at the bytecodes, you could use UNPACK_SEQUENCE with an arg of 1 on its own to do the operation. It'll verify the iterator is exhausted too afterward as a bonus, unlike next(iter(Ts)).

@stroxler
Copy link
Contributor

This looks good to me, especially if we follow @TeamSpen210's suggestion - it seems handy to verify that only length-1 iterators are ever used this way.

You're right that it's an implementation detail as long as the AST and visible behavior are nailed down, so if reviewers of the cpython code have other ideas that should also be okay.

We should be able to use the same approach for unpacking in callable syntax.

pep-0646.rst Outdated Show resolved Hide resolved
pep-0646.rst Outdated Show resolved Hide resolved
pep-0646.rst Outdated Show resolved Hide resolved
* Don't link to the current draft implementation
* Clarify the code emitted for *args: *Ts now we've switched to the UNPACK_SEQUENCE bytecode in CPython
* Update the examples of nonsensical *args annotations
@mrahtz
Copy link
Contributor Author

mrahtz commented Dec 13, 2021

Thanks for the excellent suggestion, @TeamSpec210! I've switched to UNPACK_SEQUENCE in mrahtz/cpython@81a7327 and it works like a charm :)

And thanks, Guido, for the feedback! I've made the changes you suggested.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

LGTM. I will merge now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants