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

"if 0: return" not raising SyntaxError #46183

Closed
arigo mannequin opened this issue Jan 19, 2008 · 15 comments
Closed

"if 0: return" not raising SyntaxError #46183

arigo mannequin opened this issue Jan 19, 2008 · 15 comments
Assignees
Labels
3.7 (EOL) end of life 3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@arigo
Copy link
Mannequin

arigo mannequin commented Jan 19, 2008

BPO 1875
Nosy @amauryfa, @mdickinson, @cfbolz, @benjaminp, @ned-deily, @serhiy-storchaka, @matrixise, @pablogsal, @miss-islington
PRs
  • bpo-1875: Raise SyntaxError in invalid blocks that will be optimized away #13332
  • [3.7] bpo-1875: Raise SyntaxError in invalid blocks that will be optimised away (GH-13332) #13382
  • bpo-1875, bpo-32477: Raise SyntaxError in invalid blocks that will be optimized away. #14116
  • Files
  • x.py
  • return_outside_func.patch
  • 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/amauryfa'
    closed_at = <Date 2019-12-02.22:41:47.934>
    created_at = <Date 2008-01-19.18:55:49.322>
    labels = ['interpreter-core', '3.8', 'type-bug', '3.7']
    title = '"if 0: return" not raising SyntaxError'
    updated_at = <Date 2019-12-02.22:41:47.933>
    user = 'https://github.com/arigo'

    bugs.python.org fields:

    activity = <Date 2019-12-02.22:41:47.933>
    actor = 'pablogsal'
    assignee = 'amaury.forgeotdarc'
    closed = True
    closed_date = <Date 2019-12-02.22:41:47.934>
    closer = 'pablogsal'
    components = ['Interpreter Core']
    creation = <Date 2008-01-19.18:55:49.322>
    creator = 'arigo'
    dependencies = []
    files = ['9231', '12926']
    hgrepos = []
    issue_num = 1875
    keywords = ['patch', 'needs review']
    message_count = 15.0
    messages = ['60213', '61638', '80955', '80961', '81019', '81023', '116917', '342464', '342525', '342555', '342559', '342705', '342706', '345684', '347350']
    nosy_count = 12.0
    nosy_names = ['amaury.forgeotdarc', 'mark.dickinson', 'fijal', 'Carl.Friedrich.Bolz', 'benjamin.peterson', 'ned.deily', 'Arfrever', 'mastrodomenico', 'serhiy.storchaka', 'matrixise', 'pablogsal', 'miss-islington']
    pr_nums = ['13332', '13382', '14116']
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue1875'
    versions = ['Python 3.7', 'Python 3.8']

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Jan 19, 2008

    Can you guess why importing the attached x.py does nothing, without
    printing "hello" at all?

    The real issue shown in that example is that 'return' and 'yield'
    outside a function are ignored instead of giving a SyntaxError if they
    are optimized away by 'if 0:'.

    (Hint about x.py: the yield sets the CO_GENERATOR flag before it gets
    optimized away. So clearly, that's a way to comment out a whole module
    -- just put "if 0: yield" anywhere at top-level...)

    @arigo arigo mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jan 19, 2008
    @mdickinson
    Copy link
    Member

    See also issue bpo-1920

    @amauryfa
    Copy link
    Member

    amauryfa commented Feb 2, 2009

    This was corrected by Benjamin with r69158.
    Now the yield statement is still optimized away, but at least the
    CO_GENERATOR flag is set only when compiling a function.

    @amauryfa amauryfa closed this as completed Feb 2, 2009
    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Feb 2, 2009

    ...which does not really solve anything, as "if 0: yield" at
    module-level is now just ignored instead of raising a SyntaxError (e.g.
    like "if False: yield").

    @amauryfa
    Copy link
    Member

    amauryfa commented Feb 2, 2009

    Here is a patch that properly raises SyntaxError when 'return' or
    'yield' statements appear outside a function.

    I did not bother to update the (deprecated) compiler package: it seems
    to be completely foreign to this kind of checks...

    >>> dis.dis(compiler.compile("return 1", "module.py", "exec"))
      1           0 LOAD_CONST               1 (1)
                  3 RETURN_VALUE
                  4 LOAD_CONST               0 (None)
                  7 RETURN_VALUE

    @amauryfa amauryfa reopened this Feb 2, 2009
    @amauryfa amauryfa self-assigned this Feb 2, 2009
    @benjaminp
    Copy link
    Contributor

    You should remove the error logic compile.c for "return" and "yield" in
    function calls.

    The problem with this approach is that every SyntaxError generated
    during bytecode compilation must be moved to earlier. For example, I can
    still use "break" and "continue" outside loops with your patch. The only
    way I can think of around this is to compile the block anyway and remove
    the extra code later. Maybe this optimization could be moved to the peep
    holer?

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Sep 20, 2010

    msg81023 indicates that more work is needed on this.

    @BreamoreBoy BreamoreBoy mannequin added the type-bug An unexpected behavior, bug, or error label Sep 20, 2010
    @matrixise
    Copy link
    Member

    With the last 2.7 and 3.7, I can import the x module and the 'hello' is automatically printed.

    Python 2.7.15 (default, Oct 15 2018, 15:26:09) 
    [GCC 8.2.1 20180801 (Red Hat 8.2.1-2)] on linux2
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import x
    hello
    
    
    Python 3.7.3 (default, Apr  2 2019, 06:55:20) 
    [GCC 8.3.1 20190223 (Red Hat 8.3.1-2)] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import x
    hello
    
    >>> content = open('x.py').read()
    >>> import dis
    >>> dis.dis(content)
      1           0 LOAD_NAME                0 (print)
                  2 LOAD_CONST               0 ('hello')
                  4 CALL_FUNCTION            1
                  6 POP_TOP

    3 8 LOAD_CONST 1 (None)
    10 RETURN_VALUE

    but because I am not sure about this issue, I prefer to ask Serhiy.

    Can we close it?

    Thank you

    @matrixise matrixise added 3.7 (EOL) end of life 3.8 only security fixes labels May 14, 2019
    @pablogsal
    Copy link
    Member

    The issue is not fixed. The problem is that this still allows invalid syntax because the code is optimized away:

    def f():
        if 0:
            break
        print("Hello")
    
    f()

    @serhiy-storchaka
    Copy link
    Member

    The drawback of compiling the dead code is adding cells for unneeded constants and local variables. Also it can create less optimal code for jumps.

    @pablogsal
    Copy link
    Member

    The drawback of compiling the dead code is adding cells for unneeded constants and local variables. Also it can create less optimal code for jumps.

    Is unlikely that this situation arises often enough that this is a concern. We would be sacrificing correctness for speed and I think that is an error.

    If there is a more optimal approach I'm happy to implement it.

    @pablogsal
    Copy link
    Member

    New changeset af8646c by Pablo Galindo in branch 'master':
    bpo-1875: Raise SyntaxError in invalid blocks that will be optimised away (GH-13332)
    af8646c

    @miss-islington
    Copy link
    Contributor

    New changeset 85ed171 by Miss Islington (bot) in branch '3.7':
    bpo-1875: Raise SyntaxError in invalid blocks that will be optimised away (GH-13332)
    85ed171

    @serhiy-storchaka
    Copy link
    Member

    The issue is not fixed yet. The compiler now rejects

    if 0:
        return

    but it still accepts

    if 1:
        pass
    else:
        return
    
    while 0:
        return

    @ned-deily
    Copy link
    Member

    See also discussion in bpo-37500.

    @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.7 (EOL) end of life 3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    8 participants