8000 bpo-31926: fix missing *_METHODDEF statements by argument clinic by taleinat · Pull Request #4230 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-31926: fix missing *_METHODDEF statements by argument clinic #4230

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 3, 2017
Merged

bpo-31926: fix missing *_METHODDEF statements by argument clinic #4230

merged 3 commits into from
Nov 3, 2017

Conversation

taleinat
Copy link
Contributor
@taleinat taleinat commented Nov 2, 2017 •
edited
Loading

When a single .c file contains several functions and/or methods with the same name, a safety _METHODDEF #define block is generated only for one of them.

This fixes the bug by using the full name of the function to avoid duplicates rather than just the name.

https://bugs.python.org/issue31926

@taleinat
bpo-31926: fix missing *_METHODDEF statements by argument clinic
d2aeb8f 
When a single .c file contains several functions and/or methods with
the same name, a safety _METHODDEF #define statement is generated
only for one of them.

This fixes the bug by using the full name of the function to avoid
duplicates rather than just the name.
@vstinner
Copy link
Member
vstinner commented Nov 2, 2017

You have to run "make clinic": PR must included generated files.

Running "make clinic" changes Modules/clinic/zlibmodule.c.h to add:

diff --git a/Modules/clinic/zlibmodule.c.h b/Modules/clinic/zlibmodule.c.h
index 3edf7db7c2..33c767236b 100644
--- a/Modules/clinic/zlibmodule.c.h
+++ b/Modules/clinic/zlibmodule.c.h
@@ -467,4 +467,8 @@ exit:
 #ifndef ZLIB_COMPRESS_COPY_METHODDEF
     #define ZLIB_COMPRESS_COPY_METHODDEF
 #endif /* !defined(ZLIB_COMPRESS_COPY_METHODDEF) */
-/*[clinic end generated code: output=e0184313eb431e95 input=a9049054013a1b77]*/
+
+#ifndef ZLIB_DECOMPRESS_COPY_METHODDEF
+    #define ZLIB_DECOMPRESS_COPY_METHODDEF
+#endif /* !defined(ZLIB_DECOMPRESS_COPY_METHODDEF) */
+/*[clinic end generated code: output=6378d429f0819817 input=a9049054013a1b77]*/

So I think that you can remove the #ifdef in zlibmodule.c:

static PyMethodDef Decomp_methods[] =
{
    ZLIB_DECOMPRESS_DECOMPRESS_METHODDEF
    ZLIB_DECOMPRESS_FLUSH_METHODDEF
#ifdef HAVE_ZLIB_COPY
    ZLIB_DECOMPRESS_COPY_METHODDEF
#endif
    {NULL, NULL}
};

@serhiy-storchaka
Copy link
Member

I think it is worth to add a news entry into the "Build" section.

@vstinner
Copy link
Member
vstinner commented Nov 2, 2017

Oh, and you can do the same change for ZLIB_COMPRESS_COPY_METHODDEF.

@taleinat
Copy link
Contributor Author
taleinat commented Nov 2, 2017

I've done all that @serhiy-storchaka and @Haypo suggested.

Please review the NEWS entry I wrote.

Note: This will require manual backporting.

@taleinat
implemented changes requested by reviewers
35a96e1 
* add news entry
* run "make clinic"
* remove two #ifdef checks in zlibmodule.c which are no longer needed
@taleinat taleinat force-pushed the bpo-31926_clinic_missing_methoddefs branch from fdc585a to 35a96e1 Compare November 2, 2017 21:12
@taleinat taleinat removed the skip news label Nov 2, 2017
serhiy-storchaka
serhiy-storchaka reviewed Nov 2, 2017
View reviewed changes
Misc/NEWS.d/next/Build/2017-11-02-23-10-08.bpo-31926.tbb5xH.rst Outdated
@@ -0,0 +1,4 @@
rgument Clinic failing to generate some ``#ifndef ... #define`` blocks for
Copy link
Member

Choose a reason for hiding this comment

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

✂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Can this go in?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for been too terse and ambiguous. I meant that the NEWS entry were cut (missed starting "A" and likely "Fixed"), not that it should be shortened. But in any case the new wording looks better to me!

@serhiy-storchaka serhiy-storchaka merged commit 4f57409 into python:master Nov 3, 2017
@miss-islington
Copy link
Contributor

Thanks @taleinat for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @taleinat and @serhiy-storchaka, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 4f57409a2f7bdf8fb559cddc7c6533ca2c471c67 3.6

@serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label Nov 3, 2017
@taleinat
Copy link
Contributor Author
taleinat commented Nov 3, 2017

Should I make the 3.6 backport?

@serhiy-storchaka
Copy link
Member

Yes, please do this. We will not convert new files to Argument Clinic in 3.6, and the existing files contain workarounds, but it is better to fix the bug.

taleinat added a commit to taleinat/cpython that referenced this pull request Nov 3, 2017
@taleinat
[3.6] bpo-31926: fix missing *_METHODDEF statements by argument clinic (
a77fbd2 
pythonGH-4230)

When a single .c file contains several functions and/or methods with
the same name, a safety _METHODDEF GH-define statement is generated
only for one of them.

This fixes the bug by using the full name of the function to avoid
duplicates rather than just the name..
(cherry picked from commit 4f57409)
@bedevere-bot
Copy link

GH-4253 is a backport of this pull request to the 3.6 branch.

vstinner pushed a commit that referenced this pull request Nov 3, 2017
@taleinat @vstinner
[3.6] bpo-31926: fix missing *_METHODDEF statements by argument clinic (
f8b3f6b 
GH-4230) (#4253)

When a single .c file contains several functions and/or methods with
the same name, a safety _METHODDEF GH-define statement is generated
only for one of them.

This fixes the bug by using the full name of the function to avoid
duplicates rather than just the name..
(cherry picked from commit 4f57409)
embray pushed a commit to embray/cpython that referenced this pull request Nov 9, 2017
@taleinat @embray
bpo-31926: fix missing *_METHODDEF statements by argument clinic (pyt…
0b1d3f5 
…hon#4230)

When a single .c file contains several functions and/or methods with
the same name, a safety _METHODDEF #define statement is generated
only for one of them.

This fixes the bug by using the full name of the function to avoid
duplicates rather than just the name.
@taleinat taleinat deleted the bpo-31926_clinic_missing_methoddefs branch June 25, 2018 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@larryhastings larryhastings Awaiting requested review from larryhastings

Assignees
No one assigned
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participa 39D9 nts
@taleinat @vstinner @serhiy-storchaka @miss-islington @bedevere-bot @the-knights-who-say-ni
0