#define hypot _hypot conflicts with existing definition

[tabrezm]

BPO	20221
Nosy	@mdickinson, @zware
Files	fix20221.patch

^{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:

assignee = 'https://github.com/zware'
closed_at = <Date 2014-02-20.21:42:38.996>
created_at = <Date 2014-01-11.03:09:28.607>
labels = ['build', 'OS-windows']
title = '#define hypot _hypot conflicts with existing definition'
updated_at = <Date 2014-10-17.16:12:05.024>
user = 'https://bugs.python.org/tabrezm'

bugs.python.org fields:

activity = <Date 2014-10-17.16:12:05.024>
actor = 'zach.ware'
assignee = 'zach.ware'
closed = True
closed_date = <Date 2014-02-20.21:42:38.996>
closer = 'zach.ware'
components = ['Windows']
creation = <Date 2014-01-11.03:09:28.607>
creator = 'tabrezm'
dependencies = []
files = ['33447']
hgrepos = []
issue_num = 20221
keywords = ['patch']
message_count = 17.0
messages = ['207894', '207897', '207899', '207902', '208981', '209557', '209571', '209573', '209576', '209582', '211746', '211754', '211755', '213834', '228230', '229577', '229578']
nosy_count = 6.0
nosy_names = ['mark.dickinson', 'python-dev', 'zach.ware', 'Brecht.Van.Lommel', 'tabrezm', 'Shy.Shalom']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue20221'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

[tabrezm]

In pyconfig.h (line 216), there is this line:
#define hypot _hypot

This conflicts with the definition of _hypot that ships with VS2010 (math.h, line 161):
static __inline double __CRTDECL hypot(In double _X, _In_ double _Y)

The result of the redefinition is that the following warning occurs during compilation:
1>c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\include\math.h(162): warning C4211: nonstandard extension used : redefined extern to static

When compiling with warnings being treated as errors, this will obviously result in failed builds.

The recommendation from Microsoft (see http://connect.microsoft.com/VisualStudio/feedback/details/633988/warning-in-math-h-line-162-re-nonstandard-extensions-used) is to change the definition to:
#if _MSC_VER < 1600
#define hypot _hypot
#endif

[zware]

How are you compiling to get that warning? I've never seen it, and none of the Windows buildbots do either. Also, which version of Python are you compiling on which version of Windows?

[tabrezm]

v100 toolset, with compiler setting /W4. Microsoft recommends W4 for all new C++ projects (see http://msdn.microsoft.com/en-us/library/thxezb7y(v=vs.100).aspx). I'm using 3.3.

[tabrezm]

Sorry, I realize I didn't mention this in the original post. I'm getting this compiler warning when building my Python extension, not when building Python itself.

[BrechtVanLommel]

We have a similar issue in Blender (where Python is embedded), but it's actually causing a crash instead of only a compiler warning: https://developer.blender.org/T38256

The code in the Visual Studio 2013 math.h looks like this:

__inline double __CRTDECL hypot(In double _X, _In_ double _Y)
{
return _hypot(_X, _Y);
}

With the #define hypot _hypot that becomes:

__inline double __CRTDECL _hypot(In double _X, _In_ double _Y)
{
return _hypot(_X, _Y);
}

So you get infinite recursive calls when using hypot and the application crashes. The patch fix20221.patch that was attach here solves the issue.

[zware]

I'm having difficulty wrapping my head around why the math and cmath modules (both of which use hypot) compile fine, but your extensions don't. Anyone have any insight into why that is?

[tabrezm]

My extension doesn't compile because I treat warnings as errors. I believe Blender compiles fine, but crashes at runtime because of the infinite recursion (see the second code snippet in Brecht's response (http://bugs.python.org/msg208981).

[zware]

Sorry, I wasn't entirely clear. By "compile fine", I meant "compiles without warnings, and actually works when you try to use it". Both math and cmath make use of hypot (as defined in pyconfig.h), but don't have any problems.

They also have no problems with your patch applied, but I don't feel I understand the situation enough yet to move the patch forward.

[BrechtVanLommel]

For Visual Studio 2013, here's how to redo the problem. Take this simple program:

#include <Python.h>

int main(int argc, char **argv)
{
    return (int)hypot(rand(), rand());
}

And compile it:

cl.exe test.c -I include/python3.3 lib/python33.lib /W4

c:\program files (x86)\microsoft visual studio 12.0\vc\include\math.h(566) : warning C4717: '_hypot' : recursive on all control paths, function will cause runtime stack overflow

I don't know about the conditions to get a warning in VS 2010, never tried that version.

[zware]

Your test program works for VS2010 as well (/W4 is unnecessary, the default warning level gives the warning), but still doesn't answer the question of why the math module (specifically math.hypot) doesn't show the problem.

I understand why both of your cases *don't* work, I want to understand why mathmodule.c and cmathmodule.c (and complexobject.c, for that matter) *do* work. Attempting to compile mathmodule.c alone results in the warning, and even picking through mathmodule.i as produced by preprocessing to file, it looks like math_hypot should always have the problem.

The fact that math_hypot works when compiled with the rest of Python frankly frustrates me quite a lot, because I can see no reason why it should.

[zware]

Ok, I had missed that the warnings in your two separate cases were in fact different. I still don't understand why VC++ sees the two cases so differently (throwing different warnings), but it at least explains the different results in your two cases.

I don't see how this change can actually break anything, so I'll go ahead and commit so this makes it into 3.3.5 (and hopefully 3.4.0, but that will be up to Larry Hastings).

[python-dev]

New changeset 9aedb876c2d7 by Zachary Ware in branch '3.3':
Issue bpo-20221: Removed conflicting (or circular) hypot definition
http://hg.python.org/cpython/rev/9aedb876c2d7

New changeset bf413a97f1a9 by Zachary Ware in branch 'default':
Issue bpo-20221: Removed conflicting (or circular) hypot definition
http://hg.python.org/cpython/rev/bf413a97f1a9

[zware]

Fixed, thanks for the report and patch!

[python-dev]

New changeset 033d686af4c1 by Zachary Ware in branch '3.4':
Issue bpo-20221: Removed conflicting (or circular) hypot definition
http://hg.python.org/cpython/rev/033d686af4c1

[ShyShalom]

Any chance this would be merged to 2.7 as well?
The comment from 2014-01-11 06:33:18 says "2.7" but PC/pyconfig.h still has #define hypot _hypot

[python-dev]

New changeset 430aaeaa8087 by Zachary Ware in branch '2.7':
Issue bpo-20221: Removed conflicting (or circular) hypot definition
https://hg.python.org/cpython/rev/430aaeaa8087

[zware]

It grafted very easily, so it turns out to be "yes" :)