Repository navigation
Argument clinic: add support for creating method aliases #113270
Description
Activity
How about a @ directive?
/*[clinic input]
@aliases __replace__ _replace
datetime.date.replace
args
...
[clinic start generated code]*/
Alternatively:
/*[clinic input]
@alias __replace__ as datetime_replace_alias
datetime.date.replace as datetime_replace
args
...
[clinic start generated code]*/
Possibly this is a dumb question -- how would this differ from the existing clone feature?
Possibly this is a dumb question -- how would this differ from the existing clone feature?
It only creates a new PyMethodDef entry. There is no function declaration, etc.
Got it. That'll be something to make sure we're explicit about in the devguide docs.
It is a similar concept to cloning though, so I guess I lean towards something like @eltoder's proposed syntax (something involving an assignment) rather than an @ directive?
Got it. That'll be something to make sure we're explicit about in the devguide docs.
Yeah, absolutely.
It is a similar concept to cloning though, so I guess I lean towards something like @eltoder's proposed syntax (something involving an assignment) rather than an
@directive?
Yes, it is similar to cloning. No matter what syntax we land on, I think we need to be able to support the as notation. That is, either:
/*[clinic input]
alias datetime.date.__replace__ = datetime.date.replace as date___replace__
[clinic start generated code]*/
... or a variant of #113270 (comment).
I think the decorator syntax could work and be a very compact and readable notation:
/*[clinic input]
@alias("datetime.date.__replace__")
datetime.date.replace as datetime_replace
args
...
[clinic start generated code]*/To be honest, I think the ideal syntax is just the assignment like in Python:
/*[clinic input]
datetime.date.__replace__ = datetime.date.replace
[clinic start generated code]*/and the cloning feature would be something like:
/*[clinic input]
datetime.date.bar from datetime.date.foo
[clinic start generated code]*/since it isn't an assignment. But that's probably too late to change. We can go with something like this instead:
/*[clinic input]
datetime.date.__replace__ alias datetime.date.replace
[clinic start generated code]*/In this case, we do not need to support as: there's no C implementation for the alias and the one for the target (right hand side) should have been declared in its own definition.
Actually, I take it back about as. It is needed to give the C macro and the python alias different names. Though to be consistent with the cloning syntax, as should go before =:
/*[clinic input]
alias datetime.date.__replace__ as date___replace__ = datetime.date.replace
[clinic start generated code]*/or
/*[clinic input]
datetime.date.__replace__ as date___replace__ alias datetime.date.replace
[clinic start generated code]*/Yeah, I think we need to support custom macro names with the as syntax.
Let's compare alternatives proposed so far:
Syntax
-
aliasdirective:/*[clinic input] # Declare the 'replace' function. datetime.date.replace args [clinic start generated code]*/ /*[clinic input] # Declare the alias '__replace__' in a separate clinic input block. alias datetime.date.__replace__ as date__replace__ = datetime.date.replace [clinic start generated code]*/ -
modify
state_modulename_nameto support thealiassyntax:/*[clinic input] # Declare the 'replace' function and its alias '__replace__' # in a single block, on one line. datetime.date.__replace__ as date__replace__ alias datetime.date.replace args [clinic start generated code]*/ -
@aliasannotation:/*[clinic input] # Declare the 'replace' function and its alias '__replace__' # in a single block, on two lines. @alias __replace__ as date__replace__ datetime.date.replace args [clinic start generated code]*/
I find 2. hard to read, and I dare not to think of how hard it would be to interpret such a line if you involve cloning as well 😄
Implementation
-
We'd need a new
def directive_alias()function inDSLParser. We'd have to refactorstate_modulename_name()so we could easily reuse theasand=syntax.directive_aliaswill need to perform lookups similar to the cloning feature ofstate_modulename_name(), so it will probably end up with a complexity similar to that function. -
state_modulename_name()is already fairly complex, so I would prefer to refactor it prior to adding support for thealiasclause. The implementation itself will probably be a smaller diff than 1., but there will be a lot of syntax corner cases to deal with (=thenaliasthenas?asthen=thenalias? etc.), both in the implementation and not to mention test suite. -
We'd need a new
def at_alias()function inDSLParser. We already did some refactoring regarding theas"keyword" (parse_function_names()); we could probably reuse that to implementasforat_alias(). Contrary to 2., we would not need to perform lookups to find the original declaration; we can just store the alias in the parser object and pick it up again when we enter the parsing state machine.
I believe 3. will be easiest to implement, but I may be wrong.
Proof-of-concept implementation of 3.: https://github.com/erlend-aasland/cpython/pull/new/clinic/alias
I agree that 2 seems hard to read, so 1 or 3 are better options. 3 seems like the most compact syntax and easier to implement. The only thing to note is that if you use short names instead of qualified names (i.e. @alias __replace__ instead of @alias datetime.date.__replace__) you cannot create an alias in another class. This is occasionally handy, for example, to expose a method from a base class in a derived class, e.g.
class Base:
def foo(self): ...
class Derived(Base):
base_foo = Base.foo
@override
def foo(self): ...However, I don't currently have a case like this for argument clinic.
This is occasionally handy, for example, to expose a method from a base class in a derived class, e.g.
This would require the base class and the target method to be declared using Argument Clinic. It would also require that the base class and the derived class is implemented in the same file.
This would require the base class and the target method to be declared using Argument Clinic. It would also require that the base class and the derived class is implemented in the same file.
This is the case, for example, in the datetime module, so it is not unrealistic. But I don't have a use case for cross-class aliases at the moment.
Proof-of-concept implementation of 3.: https://github.com/erlend-aasland/cpython/pull/new/clinic/alias
You can make DSLParser.alias a list. Then you can declare multiple aliases by repeating @alias directives.
You can make
DSLParser.aliasa list. Then you can declare multiple aliases by repeating@aliasdirectives.
I thought about that, but let's not over-design this from the start. If the need arises, it is fairly trivial to allow multiple aliases as a future enhancement. We developed the keyword-only/positional-only deprecation system in Argument Clinic with similar enhancement iterations.
But I don't have a use case for cross-class aliases at the moment.
If there is no use-case, we don't need to implement support for it :) If the need arises, we can deal with it in a follow-up issue.
I thought about that, but let's not over-design this from the start. If the need arises, it is fairly trivial to allow multiple aliases as a future enhancement. We developed the keyword-only/positional-only deprecation system in Argument Clinic with similar enhancement iterations.
Sure, but seems like an odd limitation. Having a list will actually make the code simpler since you can remove the "Called @alias twice!" check and turn two other ifs into fors without any other changes.
Having a list will actually make the code simpler [...]
I suggest to wait with that statement until tests have been added ;)
I suggest to wait with that statement until tests have been added ;)
You'll have to write a test for using two alias directives either way :)
I forgot one thing: we need to generate a new docstring signature for the alias, which means we need to duplicate the docstring definition as well.
I forgot one thing: we need to generate a new docstring signature for the alias, which means we need to duplicate the docstring definition as well.
^^ @AlexWaygood: this implies we need to bring back our old plan of refactoring the docstring generation. In our previous attempt, we moved the complete docstring generation to the Function object, but approach was rejected: see #107840. IIRC, we briefly discussed having a generator class; perhaps that approach can be revived; so a DocstringGenerator class. Let's create an issue for it.
@erlend-aasland looks like github is missing a yak-shaving emoji :-)
You could also just use clone, then have the implementation of the clone call the implementation of the original. Compilers are smart these days--calling a static function with identical arguments will get optimized away, so it won't be any slower. Assuming the number of places where we have these duplicate method defs is small, I'd probably prefer this clone approach, as this solves the problem with adding new syntax / implementation / conceptual load to Clinic.
Thanks for chiming in, Larry. Yeah, that's probably the best option.
You could also just use
clone, then have the implementation of the clone call the implementation of the original. Compilers are smart these days--calling a static function with identical arguments will get optimized away, so it won't be any slower.
Compilers will optimize it by just inlining the static function. This leads to code bloat as the argument parsing code, which can be substantial, and the function implementation will be duplicated in the executable. You can see this for yourself, for example: https://godbolt.org/z/GfMjfes8c. Notice how datetime_time_replace and datetime_time_replace2 are duplicated in the output. This means we are getting a bigger executable and slower compilation. It is much inferior to just creating a second PyMethodDef pointing to the same function, as was a common practice before argument clinic.
The alias feature itself seems very simple. @erlend-aasland had most of it implemented in less than a day.
@erlend-aasland Are you still working on this?
@erlend-aasland Are you still working on this?
Not actively, so feel free to pick it up. But I do think we need to do more refactorings (as previously mentioned) in order to pull this off nicely. My branch is a very limited variant of this feature.
Feature or enhancement
Proposal:
Before argument clinic, creating method aliases in C extension classes was very easy: simply add another MethodDef for the same function:
{"replace", _PyCFunction_CAST(datetime_replace), METH_VARARGS | METH_KEYWORDS}, {"__replace__", _PyCFunction_CAST(datetime_replace), METH_VARARGS | METH_KEYWORDS},This is very cheap and convenient. However, with argument clinic we cannot quite do this. If I do
I have to manually copy the flags from the generated MethodDef into the one I wrote myself. This works now, but can break later if clinic generates a MethodDef with different flags. It is possible to work-around by copying the function, but this adds a maintenance burden and generates argument parsing twice, leading to code bloat.
It would be nice to have some syntax to tell argument clinic to generate the alias MethodDef. It could look like this:
which would generate:
that can then be used as usual:
This is presently a problem for
code.__replace__, which is an alias for generatedcode.replace1. I also ran into this in #112921, where I convert datetime.*.replace methods to use argument clinic.CC @AlexWaygood @larryhastings @erlend-aasland
Has this already been discussed elsewhere?
No response given
Links to previous discussion of this feature:
No response
Footnotes
https://github.com/python/cpython/blob/893c9ccf48eacb02fa6ae93632f2d0cb6778dbb6/Objects/codeobject.c#L2180 ↩