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 name conflict #83922

Closed
isidentical opened this issue Feb 24, 2020 · 5 comments · Fixed by #104065
Closed

Argument Clinic name conflict #83922

isidentical opened this issue Feb 24, 2020 · 5 comments · Fixed by #104065
Labels
topic-argument-clinic type-feature A feature request or enhancement

Comments

@isidentical
Copy link
Sponsor Member

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
Copy link
Sponsor Member Author

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 isidentical added 3.9 only security fixes topic-argument-clinic type-feature A feature request or enhancement labels Feb 24, 2020
@isidentical
Copy link
Sponsor Member Author

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;

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.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@erlend-aasland
Copy link
Contributor

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

@erlend-aasland
Copy link
Contributor

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
Copy link
Contributor

(If you disagree, Batuhan, please open.)

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
3 participants