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
Factor out common bytes and bytearray implementation #70952
Comments
Proposed patch factors out the implementation of bytes and bytearray by moving common code in separate files. This is not new approach, the part of the code is already shared. The patch decreases the size of the source code by 862 lines. |
If I follow this correctly, then it seems like there are two
If so, then that seems reasonable to me and a nice cleanup. A few comments:
|
OK. Here is the first part of the patch. It moves common code and docstrings in bytes_methods.c.
There were conflicts with existing names stringlib_find and stringlib_rfind. |
New changeset 41969033eb9d by Serhiy Storchaka in branch 'default': |
Here is the second part of the patch. It moves common code for replace() method to template file Objects/stringlib/transmogrify.h. |
LGTM. |
New changeset f4406d746d27 by Serhiy Storchaka in branch 'default': |
Thank you Meador. Stage 3 patch is simple. It just adds guards in bytes-specific and unicode-specific template files to be sure that they are not used with wrong type. |
Stage 4 patch moves thin wrappers around functions from bytes_methods.c to Objects/stringlib/transmogrify.h. The _method_ suffix is added to some names to distinguish them from existing stringlib names. I'm not sure this is a good place. It may be better to add separate file (I'm going to move more simple wrappers here), but I don't have good name for it (bytes_methods.h is already used). Any ideas? |
New changeset c6c1882ecc77 by Serhiy Storchaka in branch 'default': |
New changeset b0087e17cd5e by Serhiy Storchaka in branch 'default': |
Serhiy, compiling 3.6 on Windows after this patch lets it start, but causes it to crash both importing tkinter and running patchcheck. Compiling 3.6 after your previous patch for bpo-27007 and the test suite runs without error. |
Checking, I see that 3 of 4 Windows buildbots ran with this patch, so there must be something significantly different about my VS install with respect to this particular patch. bpo-26624 might or might not have something to do with it (see my posts today). I can't tell from the logs which update the buildbots are running. The fourth, x86windows7 crashed before running anything (http://buildbot.python.org/all/builders/x86%20Windows7%203.x/builds/11283/steps/test/logs/stdio), after having previously run part way through the tests. It *might* have a similar problem, or might not. |
New changeset 0638047c0c36 by Serhiy Storchaka in branch 'default': |
New changeset 6c4c0a23fabb by Serhiy Storchaka in branch 'default': |
I don't know what is wrong with this patch. Backed out just for the case. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: