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

Move nargs/isva to CodeInfo #54341

Merged
merged 1 commit into from
May 11, 2024
Merged

Move nargs/isva to CodeInfo #54341

merged 1 commit into from
May 11, 2024

Commits on May 7, 2024

  1. Move nargs/isva to CodeInfo

    This changes the canonical source of truth for va handling from
    `Method` to `CodeInfo`. There are multiple goals for this change:
    
    1. This addresses a longstanding complaint about the way that
       CodeInfo-returning generated functions work. Previously, the
       va-ness or not of the returned CodeInfo always had to match that
       of the generator. For Cassette-like transforms that generally have
       one big generator function that is varargs (while then looking up
       lowered code that is not varargs), this could become quite annoying.
       It's possible to workaround, but there is really no good reason to tie
       the two together. As we observed when we implemented OpaqueClosures, the
       vararg-ness of the signature and the `vararg arguments`->`tuple`
       transformation are mostly independent concepts. With this PR, generated
       functions can return CodeInfos with whatever combination of nargs/isva
       is convenient.
    
    2. This change requires clarifying where the va processing boundary is
       in inference. #54076 was already moving in that direction for irinterp,
       and this essentially does much of the same for regular inference. As a
       consequence the constprop cache is now using non-va-cooked signatures,
       which I think is preferable.
    
    3. This further decouples codegen from the presence of a `Method` (which
       is already not assumed, since the code being generated could be a
       toplevel thunk, but some codegen features are only available to things
       that come from Methods). There are a number of upcoming features that
       will require codegen of things that are not quite method specializations
       (See design doc linked in #52797 and things like #50641). This helps
       pave the road for that.
    
    4. I've previously considered expanding the kinds of vararg signatures that
       can be described (see e.g. #53851), which also requires a decoupling of
       the signature and ast notions of vararg. This again lays the groundwork
       for that, although I have no immediate plans to implement this change.
    
    Impact wise, this adds an internal field, which is not too breaking,
    but downstream clients vary in how they construct their `CodeInfo`s and
    the current way they're doing it will likely be incorrect after this change,
    so they will require a small two-line adjustment. We should perhaps consider
    pulling out some of the more common patterns into a more stable package, since
    interface in most of the last few releases, but that's a separate issue.
    Keno committed May 7, 2024
    Configuration menu
    Copy the full SHA
    d45ef7b View commit details
    Browse the repository at this point in the history