Skip to content

Exporter generates wrong or invalid code for several formula shapes #271

Description

@fumitoh

FormulaTransformer mis-qualifies or refuses several formula shapes. In each case
below modelx itself evaluates the formula correctly, and the exported model either
fails to import, raises at evaluation, or export() aborts. None of them is caught
at export time, so the first four are only discovered when someone runs the exported
model.

These were all found while fixing an unrelated comprehension-scope defect, and all
reproduce unchanged on main at 84b155b — they are long-standing, not regressions.

Environment: modelx 0.32.0, libcst 1.6.0, Python 3.13.9 (Windows). Every case was
also checked against Python 3.12.9; unless noted, none is version-specific.


Wrong code that imports and then misbehaves

1. A Cells read inside a comprehension is left bare when a sibling comprehension binds the same name

s.ys = [1, 2]

def a(x):
    return 10 * x

def f():
    return [a for a in ys], [a(y) for y in ys]

modelx returns ([1, 2], [10, 20]). Exported:

def _f_f(self):
    return [a for a in self.ys], [a(y) for y in self.ys]

and calling it raises NameError: name 'a' is not defined.

Once any inlined comprehension in the scope binds a, the merged symtable reports
a as a local, so should_replace skips the prefix — including for the read in the
other comprehension, which at run time really does resolve to the global, because
PEP 709 isolates the first comprehension's target.

This is the mirror image of the over-qualification mode of the comprehension-scope
defect. Python 3.14 adds symtable.Symbol.is_comp_iter() / is_comp_cell(), which
would decide it directly; on 3.12 and 3.13 it needs libCST's per-scope assignment
records consulted in reverse — a name that no enclosing comprehension binds, but that
the enclosing table calls local only because a sibling comprehension binds it, has to
be resolved against the module table.

2. A nested def whose own parameter is named self captures the wrong self

def a(x):
    return 10 * x

def f(xs):
    def g(self):
        return a(self)
    return [g(x) for x in xs]

modelx returns [10, 20]. Exported:

def _f_f(self, xs):
    def g(self):
        return self.a(self)          # `self` here is g's parameter, an int
    return [g(x) for x in xs]

AttributeError: 'int' object has no attribute 'a'. The transformer inserts self.
without checking whether self is shadowed at that point.

3. Positional-only parameters get self inserted after the /

def a(x):
    return x

def f(p, /, q):
    return a(p) + q

modelx returns 3 for f(1, 2). Exported:

def _f_f(p, /, self, q):
    return self.a(p) + q

f(1, 2) then raises TypeError: _c_S._f_f() missing 1 required positional argument: 'q', and self is bound to the caller's first argument.

leave_FunctionDef builds the new parameter list as
(self_param,) + tuple(updated_node.params.params). libCST keeps pre-/ parameters
in a separate posonly_params field, which is never consulted, so self lands after
the marker instead of before it.


Code that does not import

4. global on a Cells or Reference name emits global self.NAME

def n(t):
    return t

def f():
    global n
    n = 1
    return n

modelx returns 1. Exported:

def _f_f(self):
    global self.n
    self.n = 1
    return self.n

global self.n is a SyntaxError, so importing the exported package fails and the
whole model is lost, not just this one Cells. The names in a global / nonlocal
statement are declarations, not references, and must never be qualified.

5. A bare name wrapped in its own parentheses becomes self.(name)

s.ys = [1, 2]

def f():
    return (ys)

modelx returns [1, 2]. Exported:

def _f_f(self):
    return self.(ys)

another SyntaxError at import. libCST attaches parentheses to the innermost
expression node, so (ys) parses to Name(value='ys', lpar=(LeftParen(),), rpar=(RightParen(),)), and leave_Name wraps that node in an Attribute whose
value carries the parentheses. It fires only when the parentheses wrap nothing but
the single name; (a(t) + ys) is fine.


export() itself raises

6. A generator expression or a lambda in a parameter default or an annotation aborts the export

def f(p=sum(t for t in range(3))):
    return p

def f(p=lambda t: t + 1):
    return p(1)

def f(x: sum(t for t in range(2)) = 1):
    return x

def f(x) -> sum(t for t in range(2)):
    return x

All of these evaluate in modelx. export() raises a bare AssertionError from
assert s.name == t.get_name() in adjust_scope_table_mapping, with no indication
of which formula is at fault.

libCST orders the scope of a default value or an annotation after the enclosing
FunctionScope, while symtable's DFS emits it before the function's own table, so
the positional pairing of scope and table desynchronises. A comprehension in
either position is fine, because PEP 709 leaves it no table; only the constructs that
still own one — generator expressions and lambdas — trip it.

7. PEP 695 syntax raises RuntimeError("must not happen")

def f(x):
    type Alias = int
    return x

export() raises RuntimeError: must not happen from
adjust_scope_table_mapping. Same for a generic def f[T](x: T) -> T. libCST
reports an AnnotationScope for the type-parameter list, and the else branch of
the scope/table walk rejects anything that is not a Global, Class, Function or
Comprehension scope.


Suggested remedy for the silent ones

Cases 1 to 3 produce code that imports cleanly and only fails later, somewhere that
does not point back at the formula. A validation pass at the end of
Exporter.export() would turn most of this class into a loud failure at export time:
walk the generated module with symtable and raise if any function-level symbol is
global and is neither a module-level name of the generated module nor a builtin.
That is what surfaced case 1 in the first place, and it runs over 28 lifelib models
in about a second:

import builtins, pathlib, symtable, sys

root = pathlib.Path(sys.argv[1])
BUILTINS = set(dir(builtins))
for f in sorted(root.rglob("_mx_classes.py")):
    src = f.read_text(encoding="utf-8")
    table = symtable.symtable(src, str(f), "exec")
    known = {s.get_name() for s in table.get_symbols()} | BUILTINS

    def walk(tbl, path):
        for child in tbl.get_children():
            here = path + [child.get_name()]
            if child.get_type() == "function":
                for sym in child.get_symbols():
                    if sym.is_global() and sym.get_name() not in known:
                        print(f"{f.parent.name}: {'.'.join(here)} -> {sym.get_name()}")
            walk(child, here)

    walk(table, [])

Cases 4 to 7 fail loudly already; 6 and 7 would mainly benefit from an error message
naming the formula instead of a bare assertion.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions