search

edgewall / genshi

Python toolkit for generation of output for the web
http://genshi.edgewall.org
Other
86 stars 35 forks source link

CodeType: swapped parameter order for co_kwonlyargcount and co_nlocals #52

Closed FelixSchwarz closed 2 years ago

FelixSchwarz commented 2 years ago

initially reported by @ztane in https://github.com/edgewall/genshi/issues/43#issuecomment-955814422

Since I was doing this for my own templating engine Tonnikala and was just casually browsing around, I noticed that Genshi seems to have accidentally swapped co_kwonlyargcount and co_nlocals c.f. CPython help() for code objects...

I think this does not affect Genshi on Python 3.8+ as we are using .replace() there but it still is a bug in Genshi.

Current Genshi code for Python 3: https://github.com/edgewall/genshi/blob/a23f3054b96b487215b04812c680075c5117470a/genshi/compat.py#L109-L113

Actual constructor arguments in CPython 3.5: https://github.com/python/cpython/blob/7544508f0245173bff5866aa1598c8f6cce1fc5f/Lib/types.py#L60-L65

Constructor for CPython 3.11 referenced in https://github.com/edgewall/genshi/issues/43#issuecomment-845491080

Genshi code Python 2: https://github.com/edgewall/genshi/blob/a23f3054b96b487215b04812c680075c5117470a/genshi/compat.py#L91-L94

Maybe I'm just confused right now but it seems the Python 2 constructor does not use the correct ordering as well: https://github.com/python/cpython/blob/3bbd2fad4d4a282c7a5a3169a4f497b97aeff319/Objects/codeobject.c#L50-L56

For Python 3 the constructor using positional parameters only is marked as "deprecated" so maybe we should use keyword parameters only.

FelixSchwarz commented 2 years ago

Just some quick comments:

  1. The Python 2 constructor in Genshi works fine as we are not using PyCode_New but code_new. I have a local commit which adds some docs for that.
  2. Even though the Python 3 docs mention CodeType(**kwargs) it seems like we need to use positional parameters only (Python 3.10 error message: TypeError: code() takes no keyword arguments).