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

Fix indexing of self.args when passing Python object as varargs parameter #5805

Merged
merged 1 commit into from Nov 8, 2023

Conversation

Baekalfen
Copy link
Contributor

This avoids a "Compiler crash in AnalyseExpressionsTransform", although still produces a compile error as intended.

The reason for the issue is that i comes from the length of args, where there is an additional object first in the list. The self.args is therefore offset by 1.

Bonus question: Why is this error here? I found this, as I was monkeypatching this error out. Seemingly, there's no issue passing PyObject* as a vararg parameter.

@da-woods
Copy link
Contributor

da-woods commented Nov 7, 2023

Bonus question: Why is this error here? I found this, as I was monkeypatching this error out. Seemingly, there's no issue passing PyObject* as a vararg parameter.

I think the idea is that we treat object and PyObject* as different - the former has its reference counting managed by Cython while the later is just an opaque pointer type. So you could always cast to <PyObject*> before passing. It's probably reasonable - it at least forces the caller to think about the reference counting implications of passing a Python object argument.

@Baekalfen
Copy link
Contributor Author

Understandable. I'm using Cython only in Pure Python Mode with pxd's, so I can't just cast to PyObject*. I'll probably keep my workaround for now, as just use it for logging, and don't really need to ref-count.

@da-woods
Copy link
Contributor

da-woods commented Nov 7, 2023

This looks good to me - thanks.

I suspect this should go in the next 3.0 bugfix release. But we're currently in the process of branching off the new development branch so I'm not 100% sure where it should go, so I'll wait a little bit to see if there's any objections.

@da-woods da-woods changed the base branch from master to 3.0.x November 8, 2023 20:08
@da-woods da-woods changed the base branch from 3.0.x to master November 8, 2023 20:08
@da-woods da-woods merged commit c016ea6 into cython:master Nov 8, 2023
64 checks passed
@da-woods
Copy link
Contributor

da-woods commented Nov 8, 2023

Thanks

@Baekalfen Baekalfen deleted the fix_arg_index branch November 8, 2023 20:09
@da-woods
Copy link
Contributor

da-woods commented Nov 8, 2023

Backported to 3.0.x in 101b492

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants