◐ Shell
reader mode source ↗
Skip to content

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

Merged
serhiy-storchaka merged 3 commits into
python:masterfrom
taleinat:bpo-31926_clinic_missing_methoddefs
Nov 3, 2017
Merged

bpo-31926: fix missing *_METHODDEF statements by argument clinic#4230
serhiy-storchaka merged 3 commits into
python:masterfrom
taleinat:bpo-31926_clinic_missing_methoddefs

Conversation

@taleinat

@taleinat taleinat commented Nov 2, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

vstinner commented Nov 2, 2017

Copy link
Copy Markdown
Member

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

serhiy-storchaka commented Nov 2, 2017

Copy link
Copy Markdown
Member

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

@vstinner

vstinner commented Nov 2, 2017

Copy link
Copy Markdown
Member

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

@taleinat

taleinat commented Nov 2, 2017

Copy link
Copy Markdown
Contributor Author

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 merged commit 4f57409 into python:master Nov 3, 2017
@miss-islington

miss-islington commented Nov 3, 2017

Copy link
Copy Markdown
Contributor

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

@bedevere-bot bedevere-bot removed the label Nov 3, 2017
@miss-islington

miss-islington commented Nov 3, 2017

Copy link
Copy Markdown
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 An unexpected behavior, bug, or error label Nov 3, 2017
@taleinat

taleinat commented Nov 3, 2017

Copy link
Copy Markdown
Contributor Author

Should I make the 3.6 backport?

@serhiy-storchaka

serhiy-storchaka commented Nov 3, 2017

Copy link
Copy Markdown
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

bedevere-bot commented Nov 3, 2017

Copy link
Copy Markdown

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@taleinat @vstinner @serhiy-storchaka @miss-islington @bedevere-bot @the-knights-who-say-ni