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

Multiple context expressions do not support parentheses for continuation across lines #56991

Closed
Julian mannequin opened this issue Aug 19, 2011 · 29 comments
Closed

Multiple context expressions do not support parentheses for continuation across lines #56991

Julian mannequin opened this issue Aug 19, 2011 · 29 comments
Assignees
Labels
3.10 interpreter-core type-feature

Comments

@Julian
Copy link
Mannequin

@Julian Julian mannequin commented Aug 19, 2011

BPO 12782
Nosy @gvanrossum, @warsaw, @birkenfeld, @atsuoishimoto, @ncoghlan, @benjaminp, @ezio-melotti, @merwok, @stevendaprano, @bitdancer, @ambv, @Julian, @serhiy-storchaka, @asottile, @pablogsal, @thautwarm, @isidentical, @websurfer5, @jack1142

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/pablogsal'
closed_at = <Date 2020-06-25.19:11:20.497>
created_at = <Date 2011-08-19.02:10:17.157>
labels = ['interpreter-core', 'type-feature', '3.10']
title = 'Multiple context expressions do not support parentheses for continuation across lines'
updated_at = <Date 2022-01-16.22:14:02.746>
user = 'https://github.com/Julian'

bugs.python.org fields:

activity = <Date 2022-01-16.22:14:02.746>
actor = 'eric.araujo'
assignee = 'pablogsal'
closed = True
closed_date = <Date 2020-06-25.19:11:20.497>
closer = 'pablogsal'
components = ['Interpreter Core']
creation = <Date 2011-08-19.02:10:17.157>
creator = 'Julian'
dependencies = []
files = []
hgrepos = []
issue_num = 12782
keywords = ['patch']
message_count = 29.0
messages = ['142411', '142549', '142550', '142631', '142658', '142750', '180512', '180514', '180515', '236083', '236121', '293660', '326637', '326639', '326645', '326648', '326727', '327875', '346137', '346157', '347025', '363755', '372383', '372384', '372385', '372388', '410726', '410728', '410729']
nosy_count = 21.0
nosy_names = ['gvanrossum', 'barry', 'georg.brandl', 'ishimoto', 'ncoghlan', 'benjamin.peterson', 'ezio.melotti', 'eric.araujo', 'steven.daprano', 'r.david.murray', 'lukasz.langa', 'Julian', 'serhiy.storchaka', 'ulope', 'Anthony Sottile', 'pablogsal', 'thautwarm', 'BTaskaya', 'Terry Davis', 'Jeffrey.Kintscher', 'jack1142']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue12782'
versions = ['Python 3.10']

@Julian
Copy link
Mannequin Author

@Julian Julian mannequin commented Aug 19, 2011

Using multiple with statements across multiple lines does not support using parens to break them up:

with (open("a_really_long_foo") as foo,
      open("a_really_long_bar") as bar):
    pass
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "demo.py", line 19
    with (open("a_really_long_foo") as foo,
                                    ^
SyntaxError: invalid syntax

Also, without convoluting things, import also does not support doing so, and is the only other example I can think of of a compound statement that forces you to either be redundant or bite your teeth and use \, despite the fact that PEP-328 gave us parens for from imports.

(I did not find a discussion as to why import didn't grow it as well, so please correct me as I'm sure it must have been discussed before).

It's understandably a lot rarer to need multiple lines when importing, but it'd be nice if all compound statements uniformly allowed the same continuation syntax.

@Julian Julian mannequin added type-bug interpreter-core labels Aug 19, 2011
@pitrou pitrou added type-feature and removed type-bug labels Aug 19, 2011
@merwok
Copy link
Member

@merwok merwok commented Aug 20, 2011

I agree it’s unfortunate that we have to use backslashes to have multi-line with statements.

@birkenfeld
Copy link
Member

@birkenfeld birkenfeld commented Aug 20, 2011

is the only other example I can think of

One similar example would be "raise" in Python 2.

all compound statements uniformly allowed the same continuation syntax.

This is not true: only "import-as" allows this syntax. All other uses of parentheses for continuation are continuations of *expressions*.

@bitdancer
Copy link
Member

@bitdancer bitdancer commented Aug 21, 2011

Following on to Georg's comment about expressions, as a workaround, note
that:

  with (
     open('abc')) as foo:

works.

@ncoghlan
Copy link
Contributor

@ncoghlan ncoghlan commented Aug 22, 2011

As Georg noted, only individual expressions get parentheses based continuations automatically. For statement level use of comma separation, it's decided on a case-by-cases basis as to whether we think it is a legitimate usage based on our style guidelines.

That's why 'from location import (name1, name2)' is allowed, but 'import (name1, name2)' is not: we explicitly advise against importing too many modules in a single import statement, but importing multiple names from a single location is often a useful thing to do.

However, while the multiple context expression use case is reasonable, there may be a grammar ambiguity problem in this case, since (unlike from-import) with statements allow arbitrary subexpressions.

@Julian
Copy link
Mannequin Author

@Julian Julian mannequin commented Aug 22, 2011

we explicitly advise against importing too many modules in a single import statement, but importing multiple names from a single location is often a useful thing to do.

Cool. I imagined this had to do with it.

there may be a grammar ambiguity problem in this case, since (unlike from-import) with statements allow arbitrary subexpressions.

Sorry, can you possibly clarify where the ambiguity might come in?

@atsuoishimoto
Copy link
Mannequin

@atsuoishimoto atsuoishimoto mannequin commented Jan 24, 2013

In Python 3.3, we have contextlib.ExitStack() for multiple contexts.
So, perhaps we can close this issue?

@ezio-melotti
Copy link
Member

@ezio-melotti ezio-melotti commented Jan 24, 2013

If this can't be fixed I think it should be at least documented in the FAQs.

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jan 24, 2013

@warsaw
Copy link
Member

@warsaw warsaw commented Feb 17, 2015

Let's just Won't Fix this. Use a contextlib.ExitStack.

@warsaw warsaw closed this as completed Feb 17, 2015
@ulope
Copy link
Mannequin

@ulope ulope mannequin commented May 14, 2017

So this would basically be:

    with ExitStack() as stack:
        cm1 = stack.enter_context(a_long_name.with_a_long_method())
        cm2 = stack.enter_context(another_long_variable.with_a_long_method())

Seems like a very non-obvious and inelegant solution...

@ambv
Copy link
Contributor

@ambv ambv commented Sep 28, 2018

This was closed without enough explanation. Suggesting people should use ExitStack due to a Python grammar deficiency is suboptimal to say the least.

This problem is coming back to users of Black due to Black's removal of backslashes. It's the only piece of our grammar where backslashes are required for readability which shows there's something wrong.

The syntax ambiguity that Nick is raising fortunately shouldn't be a problem because a single tuple is an invalid context manager. In other contexts if the organizational parentheses are matched by the with-statement and not by the underlying test, that's also fine since they were organizational this doesn't make the test invalid.

Pablo has a working patch for this, we intend to fix this wart for Python 3.8.

@ambv ambv added the 3.8 label Sep 28, 2018
@ambv ambv reopened this Sep 28, 2018
@ncoghlan
Copy link
Contributor

@ncoghlan ncoghlan commented Sep 28, 2018

Especially since the dynamic flexibility of ExitStack comes at a genuine runtime cost when unwinding the resource stack.

I also (very!) belatedly noticed that I never answered Julian's request for clarification about the potential grammar ambiguity, so going into detail about that now:

The first item in the grammar after the 'with' keyword is a 'test' node, which can already start with a parenthesis, which means a naive attempt at allowing grouping parentheses will likely fail to generate a valid LL(1) parser.

That doesn't mean a more sophisticated change isn't possible (and Pablo has apparently implemented one) - it just means that the required grammar update is going to be more complicated than just changing:

with_stmt: 'with' with_item (',' with_item)*  ':' suite

to be:

with_stmt: 'with' (with_items | '(' with_items ')') ':' suite
with_items: with_item (',' with_item)*

(That would need too much lookahead to decide whether an opening parenthesis belongs to the first 'with_item' in 'with_items' or if it's starting the alternative multi-line grouping construct)

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Sep 28, 2018

The Python grammar is already not LL(1) strictly. Take for example the production for "argument":

argument: ( test [comp_for] | test '=' test | '**' test | '*' test )

obviously the first sets of test and test are the same and is ambiguous, but the NDFAs are still able to produce DFAs that can generate a concrete syntax tree that allows the AST generation to disambiguate that the second test is a NAME and not any other thing.

The rule with_stmt: 'with' ( with_item (',' with_item)* | '(' with_item (',' with_item)* [','] ')' ) ':' suite

will generate a similar scenario. The NDFAs will generate DFAs that will ultimately allow us to just skip the more external group of parenthesis when generating the nodes. This makes valid all these expressions:

         with (manager() as x, manager() as y):
             pass
         with (manager() as x, manager() as y,):
             pass
         with (manager()):
             pass
         with (manager() as x):
             pass
         with (((manager()))):
             pass
         with ((((manager()))) as x):

but not this one:

         with (((manager()))) as x:

the reason is that it assigns the first LPAR to the second production and it fails when searching for the one that is at the end. I think this limitation is OK.

If you want to play with that. here is a prototype of the implementation with some tests:

https://github.com/pablogsal/cpython/tree/parenthesized_with

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Sep 28, 2018

The DFA for the rule

with_stmt: 'with' ( with_item (',' with_item)* | '(' with_item (',' with_item)* [','] ')' ) ':' suite

is:

DFA for with_stmt [512/2103]
State 0
'with' -> 1
State 1
'(' -> 2
with_item -> 3
State 2
with_item -> 4
State 3
',' -> 5
':' -> 6
State 4
')' -> 7
',' -> 8
State 5
with_item -> 3
State 6
suite -> 10
State 7
':' -> 6
State 8
')' -> 7
with_item -> 4
State 9
',' -> 5
':' -> 6
State 10 (final)
State 11
')' -> 7
',' -> 8

It works because the transition from State 1 into a "(" is going to prioritize the path:

0 -> 1 -> "(" -> 2

instead if

0 -> 1 -> with_item -> 3

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Sep 30, 2018

Now that I think about this a bit better, this may be actually a problem as:

         with (yield something) as x:

is a more than valid use case that will be broken with the simple grammar rule :(

@thautwarm
Copy link
Mannequin

@thautwarm thautwarm mannequin commented Oct 17, 2018

How about:

with_stmt: 'with' (with_items | '(' with_items ')') ':' suite
ignored: INDENT | NEWLINE | DEDENT
with_items: with_item (ignored* ',' ignored* with_item)*

@TerryDavis
Copy link
Mannequin

@TerryDavis TerryDavis mannequin commented Jun 20, 2019

I'd like to re-raise this issue. Should I cross-post to "discuss.python.org - Ideas"?

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Jun 20, 2019

What do you mean with re-raise? The issue is not closed. If you have some proposal to overcome the limitations, the best approach is to comment here in the issue.

@ncoghlan
Copy link
Contributor

@ncoghlan ncoghlan commented Jul 1, 2019

Reviewing the thread, we never actually commented on thautwarm's proposal in https://bugs.python.org/issue12782#msg327875 that aims to strip out any INDENT, NEWLINE, and DEDENT tokens that appear between the opening "with" keyword and the statement header terminating ":".

The problem with that is that line continuations are actually handled by the tokenizer, *not* the compiler, and the tokenizer already switches off the INDENT/NEWLINE/DEDENT token generation based on the following rules:

  • tracking opening & closing of triple-quoted strings
  • tracking opening & closing of parentheses ("()"), brackets ("[]"), and braces ("{}")
  • detecting a backslash immediately followed by a newline

By design, the tokenizer is generally unaware of which NAME tokens are actually keywords - it's only aware of async & await at the moment as part of the backwards compatibility dance that allowed those to be gradually converted to full keywords over the course of a couple of releases.

Hence why INDENT/NEWLINE/DEDENT never appear inside expressions in the Grammar: the tokenization rules mean that those tokens will never appear in those locations.

And it isn't simply a matter of making the tokenizer aware of the combination of "with" and ":" as a new pairing that ignores linebreaks between them, as ":" can appear in many subexpressions (e.g. lambda functions, slice notation, and the new assignments expressions), and it's only the full parser that has enough context to tell which colon is the one that actually ends the statement header.

Thus the design requirement is to come up with a grammar rule that allows this existing code to continue to compile and run correctly:

>>> from contextlib import nullcontext
>>> with (nullcontext()) as example:
...     pass
... 
>>> 

While also enabling new code constructs like the following:

    with (nullcontext() as example):
        pass

    with (nullcontext(), nullcontext()):
        pass

    with (nullcontext() as example, nullcontext()):
        pass

    with (nullcontext(), nullcontext() as example):
        pass

    with (nullcontext() as example1, nullcontext() as example2):
        pass

If we can get the Grammar to allow those additional placements of parentheses, then the existing tokenizer will take care of the rest.

@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Mar 9, 2020

If we introduce a PEG-based parser, we can do this without hacking the tokenizer. See we-like-parsers/pegen_experiments#229

I'd propose to aim for Python 3.10 (if the PEG parser happens).

@gvanrossum gvanrossum removed the 3.8 label Mar 9, 2020
@thautwarm
Copy link
Mannequin

@thautwarm thautwarm mannequin commented Jun 25, 2020

I can confirm Guido's words, now parentheses for continuation across lines are already supported.

Even without parentheses, multiline with items can be supported. I just implemented it here: https://github.com/thautwarm/cpython/blob/bpo-12782/Grammar/python.gram#L180-L187

  from contextlib import contextmanager

  @contextmanager
  def f(x):
    try:
        yield x
    finally:
        pass

# Ok
  with f('c') as a,
       f('a') as b:
       pass


# Ok
  with f('c') as a,
       f('a') as b,
       f('a') as c:
       pass


  # ERROR
  with f('c') as a,
       f('a') as b,
       f('a') as c:
     x = 1 + 1

# message:
File "/home/thaut/github/cpython/../a.py", line 49
x = 1 + 1
^
IndentationError: unindent does not match any outer indentation
level

  # ERROR
  with f('c') as a,
       f('a') as b,
       f('a') as c:
          x = 1 + 1

File "/home/thaut/github/cpython/../a.py", line 49
x = 1 + 1

IndentationError: unexpected indent

The grammar is:

with_stmt[stmt_ty]:
| ...
| 'with' a=(',' [NEWLINE ~ INDENT?]).with_item+ ':' tc=[TYPE_COMMENT] NEWLINE b=statements DEDENT {
_Py_With(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA) }
| ...

The restriction here is, since the second 'with_item', until the end of 'statements', the expression and statements have to keep the same indentation.

    with item1,
       item2,
       ...:
       block
    
    The indentation of 'item2', ..., 'block' should be the same.

This implementation leverages the new PEG and how the lexer deals with indent/dedent.

@thautwarm
Copy link
Mannequin

@thautwarm thautwarm mannequin commented Jun 25, 2020

Maybe you should close this.

@pablogsal
Copy link
Member

@pablogsal pablogsal commented Jun 25, 2020

This is already implemented in master with the new PEG parser so closing this.

@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Jun 25, 2020

But it is undocumented and doesn’t work with -X oldparser.

--Guido (mobile)

@ahmedsayeed1982 ahmedsayeed1982 mannequin added expert-IDLE 3.8 and removed interpreter-core labels Nov 4, 2021
@eryksun eryksun added interpreter-core and removed expert-IDLE labels Nov 4, 2021
@merwok
Copy link
Member

@merwok merwok commented Jan 16, 2022

PEP-8 used with statements as an example of use of backslashes, I am proposing this change: python/peps#2244

@merwok merwok added 3.10 and removed 3.8 labels Jan 16, 2022
@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Jan 16, 2022

[Meta: Why did adding a comment add all those people (back?) to the nosy list?]

@merwok
Copy link
Member

@merwok merwok commented Jan 16, 2022

[There are two separate events in the ticket log: I added my comment, then noticed I was the only nosy so I found all the prople recently removed by error and re-added them]

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.10 interpreter-core type-feature
Projects
None yet
Development

No branches or pull requests