Skip to content
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

Redefined specialization in type_traits.h using MSVC (Protobuf 3.1.0) #2202

Closed
mrry opened this issue Sep 29, 2016 · 2 comments
Closed

Redefined specialization in type_traits.h using MSVC (Protobuf 3.1.0) #2202

mrry opened this issue Sep 29, 2016 · 2 comments
Assignees

Comments

@mrry
Copy link
Contributor

mrry commented Sep 29, 2016

This commit added explicit specializations for is_integral<__int64> and is_integral<unsigned __int64 when compiling with Visual C++: 3c88e1b

It seems that by default HAVE_LONG_LONG is not defined on this platform, so these specializations do not collide with the specializations for is_integral<long long> and is_integral<unsigned long long> immediately above them.

However, other projects use HAVE_LONG_LONG on Windows, which leads to an error about the specialization already being defined. In particular, when I include <Python.h> from Anaconda for Windows before a header that includes some protobuf headers, I get the following compilation error:

[...]google/protobuf/stubs/type_traits.h(151): error C2766: explicit specialization; 'google::protobuf::internal::is_integral<__int64>' has already been defined
[...]google/protobuf/stubs/type_traits.h(143): note: see previous definition of 'is_integral<__int64>'
[...]google/protobuf/stubs/type_traits.h(152): error C2766: explicit specialization; 'google::protobuf::internal::is_integral<unsigned __int64>' has already been defined
[...]google/protobuf/stubs/type_traits.h(144): note: see previous definition of 'is_integral<unsigned __int64>'

Here's a simple repro, test.cc:

//#include <Python.h>  // The following #define has the same effect.
#define HAVE_LONG_LONG
#include "google/protobuf/stubs/type_traits.h"

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

Reproduced using the following command line with Visual Studio 2015 Update 1:

C:\temp> cl.exe test.cc /I<path\to\protobuf\src\>

Could the conditional compilation be refactored to avoid this problem? @xfxyjwf, since you made the commit, any ideas?

@xfxyjwf xfxyjwf self-assigned this Sep 29, 2016
@xfxyjwf
Copy link
Contributor

xfxyjwf commented Sep 29, 2016

When reading https://msdn.microsoft.com/en-us/library/29dh1w7z.aspx, it says:

The __int64 type has no ANSI equivalent.

That made me think __int64 is different from long long on MSVC. Apparently it's either my reading is wrong or the MSVC doc is lying.

I guess we can change it to:

#if defined(HAVE_LONG_LONG) || defined(_MSC_VER)
template<> struct is_integral<long long> : true_type { };
template<> struct is_integral<unsigned long long> : true_type { };
#endif

Or maybe just drop the #ifdef/#endif entirely as compilers that don't support "long long" are probably too old to compile protobuf code any way.

Can you help send a pull request to fix that?

@mrry
Copy link
Contributor Author

mrry commented Sep 29, 2016

Thanks for the quick response! Yes, I'll send a quick Pull Request with your first suggested change (after testing it with the code I'm building).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants