GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||
GeezerVison is showing github.com — gh-111178: fix UBSan failures in `Objects/complexobject.c` by picnixz · Pull Request #128241 · python/cpython Conversation
|
| #include "pycore_pymath.h" // _Py_ADJUST_ERANGE2() | ||
|
|
||
|
|
||
| #define _PyComplexObject_CAST(op) ((PyComplexObject *)(op)) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I appreciate this cleanup, but, perhaps, you should change other type casts too. E.g. in the COMPLEX_BINOP macro.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I wanted to keep a small diff but I can do that (as you can see, I've opened 20 PRs doing the same mechanical changes...)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
If you do this mechanically (i.e. with sed), I would guess you should take some extra steps to prevent other changes in same file.
Perhaps, it's better to not introduce a new macro and stick with (PyComplexObject *) casts. Or change everything.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
When Victor and I started fixing #111178, we introduced a macro so that we can (in the future), possibly add type-checks easily. By "mechanical" changes, I manually changed everything (but everytime I repeated the same steps)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| (binaryfunc)complex_add, /* nb_add */ | ||
| (binaryfunc)complex_sub, /* nb_subtract */ | ||
| (binaryfunc)complex_mul, /* nb_multiply */ | ||
| complex_add, /* nb_add */ |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This structure modified a lot. Maybe it's an opportunity to change it to include only added entries in style:
static PyNumberMethods complex_as_number = { .nb_add = complex_add, .nb_subtract = complex_sub, .nb_multiply = complex_mul, .nb_power = complex_pow, [...] };
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I wanted to do that as per #127679 but since Serhiy wanted a discussion first (#127679 (comment)) and since others wanted PEP-7 to be updated, I decided not to do that kind of cosmetic change (I admit that there are other cosmetic changes that I've done but those wouldn't cause conflicts or require a discussion IMO, they're just for consistency)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I see no discussion opened so far.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Oh.. then.. maybe I can do the change? well.. I actually did it for internal structures, but for exported ones, I don't think I did it in any of the other PR. I'll ask on the issue.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I can do the change?
Well, discussion is not started so far. That's all:)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
No need to expand the PR's scope. Feel free to send a follow-up PR.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.