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

Argument Clinic: add support for changing the scope of the generated parser #94616

Closed
erlend-aasland opened this issue Jul 6, 2022 · 1 comment
Labels
topic-argument-clinic type-feature A feature request or enhancement

Comments

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Jul 6, 2022

Feature or enhancement

Currently, AC will generate both the parser and the corresponding impl method with static scope. This is also what is desired most of the time, and it fits the general use-case in the CPython code base. Suggesting to add the possibility of generating extern parsers with the following syntax:

[extern] modulename.fnname [as c_basename] [-> return annotation]

If the line starts with the optional extern keyword, extern is used instead of static when generating the parser code.

Pitch

In the sqlite3 extension module, the code is split between several files. Module level functions are found in Modules/_sqlite/module.c, the connection object and its method in Modules/_sqlite/connection.c, etc. The sqlite3.connect() function and the sqlite3.Connection.__init__() method have identical signatures, and the former is implicitly relayed to the latter. Currently, this is very inconvenient:

  • the signatures must be equal, and I cannot use the "clone" feature of AC:
    • cloning can only be performed within a single file
    • AC cannot clone a module level function to a class method, or the other way around
  • I cannot reuse custom converters conveniently, since not everything is in one file
  • it is not possible to move the connection method to module.c and vice versa:
  • relaying the call from sqlite3.connect() to sqlite3.Connection.__init__() is currently very clumsy (take a look); having both in the same file, will make things a lot easier in a lot of ways, not to mention the call itself

If it was possible to generate extern parsers, I could move sqlite3.connect() from module.c to connection.c, redirect the methoddef, doc, and parser declarations to a header file for inclusion in module.c, and include the rest of the generated output in connection.c. I could then easily reuse the custom converter (that currently lives in connection.c), and the relay call would be a simple C function call (not a PyObject_Call*).

If I were to add a keyword-only parameter to sqlite3.connect, the relay call will be even more clumsy (and slow). I'm planning to add a keyword-only parameter in #93823. With this change, adding a keyword-only parameter will be trivial.

Other solutions

Another possibility is of course to remove AC from sqlite3.connect(), manually copy the docstring, and just use PyObject_Call(factory, args, kwds) to relay the call. I'd still have to parse the incoming args/kwds in order to fetch the factory function thought, so this option is also pretty clumsy.

@erlend-aasland erlend-aasland added type-feature A feature request or enhancement topic-argument-clinic labels Jul 6, 2022
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Jul 22, 2022

After a series of experiments, I realise that this feature won't be able to help sqlite3 :) Closing this for now; AFAIK there has never been other requests for this, and there haven't been any reactions to this issue either.

@erlend-aasland erlend-aasland closed this as not planned Won't fix, can't repro, duplicate, stale Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-argument-clinic type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

1 participant