Skip to content

[BUG]: Avoiding promoter overrides leading to global state corruption#78

Open
SwayamInSync wants to merge 4 commits intonumpy:mainfrom
SwayamInSync:promoter-fix
Open

[BUG]: Avoiding promoter overrides leading to global state corruption#78
SwayamInSync wants to merge 4 commits intonumpy:mainfrom
SwayamInSync:promoter-fix

Conversation

@SwayamInSync
Copy link
Copy Markdown
Member

closes #76

Copy link
Copy Markdown
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM, but the ld promoter doesn't really look right. I.e. it doesn't do anything interesting?

To be fair, that seems like an existing bug...


// Preserve the integer type for the exponent (slot 1)
Py_INCREF(op_dtypes[1]);
new_op_dtypes[1] = op_dtypes[1];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you want to promote to PyArray_PyLongDType here. (assuming signature[1] == NULL).
(I guess you could add more logic in case of non-ints, but maybe doesn't matter much in practice... cast safety may even reject it normally anyway.)

Although long seems weird compared to just using intp that always works, but OK. Otherwise this looks like it doesn't do anything really.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I updated the loop registration + promoter to use intp

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Promoters are matching too broadly

2 participants