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

get_var_name reports wrong name on Python 3.11 #792

Closed
Jimx- opened this issue May 23, 2023 · 0 comments · Fixed by #798
Closed

get_var_name reports wrong name on Python 3.11 #792

Jimx- opened this issue May 23, 2023 · 0 comments · Fixed by #798
Labels
Milestone

Comments

@Jimx-
Copy link
Contributor

Jimx- commented May 23, 2023

class Test(Elaboratable):

    def __init__(self):
        self.i = Signal(4)

    def elaborate(self, platform):
        m = Module()

        a = Signal()
        print(a.name)

        b = Cat(self.i[i] & a for i in range(4))
        c = Signal(4)
        m.d.sync += c.eq(b)

        return m

Expected output is "a", but on Python 3.11, it prints "self". This happens when both self and the variable are cell vars (e.g., when they are used in a generator expression).

            name_index = int(code.co_code[index + 1])
            if sys.version_info >= (3, 11):
                name_index -= code.co_nlocals
            return code.co_cellvars[name_index]

The line name_index -= code.co_nlocals assumes that code.co_cellvars does not contains any local variable but this is not the case if self is also a cell var. The code object provides a _varname_from_oparg internal method and I wonder if it is a better choice for inferring variable names.

@whitequark whitequark added the bug label May 23, 2023
@whitequark whitequark added this to the 0.4 milestone May 23, 2023
mwkmwkmwk added a commit to mwkmwkmwk/amaranth that referenced this issue Jun 1, 2023
This fixes the following issues:

- on Python 3.10 and earlier, storing to free variables is now handled
  correctly
- on Python 3.11, `_varname_from_oparg` is now used, fixing problems
  with cell variables that are also arguments
- on all supported versions, EXTENDED_ARG is now parsed, ensuring proper
  handling for long functions

Fixes amaranth-lang#792.
whitequark pushed a commit that referenced this issue Jun 1, 2023
This fixes the following issues:

- on Python 3.10 and earlier, storing to free variables is now handled
  correctly
- on Python 3.11, `_varname_from_oparg` is now used, fixing problems
  with cell variables that are also arguments
- on all supported versions, EXTENDED_ARG is now parsed, ensuring proper
  handling for long functions

Fixes #792.
lethalbit pushed a commit to shrine-maiden-heavy-industries/torii-hdl that referenced this issue Jun 10, 2023
This fixes the following issues:

- on Python 3.10 and earlier, storing to free variables is now handled
  correctly
- on Python 3.11, `_varname_from_oparg` is now used, fixing problems
  with cell variables that are also arguments
- on all supported versions, EXTENDED_ARG is now parsed, ensuring proper
  handling for long functions

Fixes amaranth-lang#792.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants