Argument Clinic name conflict

[isidentical]

BPO	39741
Nosy	@larryhastings, @pablogsal, @isidentical

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-02-24.17:30:10.453>
labels = ['type-feature', 'expert-argument-clinic', '3.9']
title = 'Argument Clinic name conflict'
updated_at = <Date 2020-02-24.20:07:45.911>
user = 'https://github.com/isidentical'

bugs.python.org fields:

activity = <Date 2020-02-24.20:07:45.911>
actor = 'BTaskaya'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Argument Clinic']
creation = <Date 2020-02-24.17:30:10.453>
creator = 'BTaskaya'
dependencies = []
files = []
hgrepos = []
issue_num = 39741
keywords = []
message_count = 2.0
messages = ['362599', '362602']
nosy_count = 3.0
nosy_names = ['larry', 'pablogsal', 'BTaskaya']
pr_nums = []
priority = 'low'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue39741'
versions = ['Python 3.9']

[isidentical]

Argument clinic uses some extra variables (like args, or noptargs, nargs etc.) for parsing. But there is a catch about these names, the generated code becomes wrong if there are any usages of them inside the signature. Encountered with this problem while working on *args support (in bpo-20291).

The possible solution is prefixing every argument in the parser with __clinic_ (_clinic{var}) for preventing any kind of conflict. I'll draft a PR for this issue.

[isidentical]

After preparing the patch and transforming all arguments with a __clinic_ prefix, I saw there are some actions that are taken by relying on the parser code. An example;

cpython/Objects/codeobject.c

Lines 616 to 631 in 8af4712

	co_argcount: int(c_default="self->co_argcount") = -1
	co_posonlyargcount: int(c_default="self->co_posonlyargcount") = -1
	co_kwonlyargcount: int(c_default="self->co_kwonlyargcount") = -1
	co_nlocals: int(c_default="self->co_nlocals") = -1
	co_stacksize: int(c_default="self->co_stacksize") = -1
	co_flags: int(c_default="self->co_flags") = -1
	co_firstlineno: int(c_default="self->co_firstlineno") = -1
	co_code: PyBytesObject(c_default="(PyBytesObject *)self->co_code") = None
	co_consts: object(subclass_of="&PyTuple_Type", c_default="self->co_consts") = None
	co_names: object(subclass_of="&PyTuple_Type", c_default="self->co_names") = None
	co_varnames: object(subclass_of="&PyTuple_Type", c_default="self->co_varnames") = None
	co_freevars: object(subclass_of="&PyTuple_Type", c_default="self->co_freevars") = None
	co_cellvars: object(subclass_of="&PyTuple_Type", c_default="self->co_cellvars") = None
	co_filename: unicode(c_default="self->co_filename") = None
	co_name: unicode(c_default="self->co_name") = None
	co_lnotab: PyBytesObject(c_default="(PyBytesObject *)self->co_lnotab") = None

In that case, self was already replaced with __clinic_self so that code doesn't work. IMHO there should be an identifier to distinguish these cases like; <self>->co_argc etc. When we see such defaults we can just replace them with the prefixed version.

[erlend-aasland]

Related to gh-68395. Can we close this after gh-104065, or do you want to follow another strategy?

[erlend-aasland]

FTR, if you try to use params named "self" in a class method (or "module" in a module level function), AC will bail and ask you to explicitly choose a custom name for the C variable. See #68395 (comment).

I think we can close this after gh-104065 lands.

[erlend-aasland]

(If you disagree, Batuhan, please open.)