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

eabase.h: error C2371: 'char16_t' : redefinition; different basic types #12

Closed
Ono-Sendai opened this Issue Feb 11, 2016 · 15 comments

Comments

Projects
None yet
7 participants
@Ono-Sendai

Ono-Sendai commented Feb 11, 2016

Hi, I'm getting the compile error below.

I added the following include paths:
D:\programming\EASTL-master\test\packages\EABase\include\Common
D:\programming\EASTL-master\include

and this line:

include < EASTL/sort.h >

and got:

1>D:\programming\EASTL-master\test\packages\EABase\include\Common\EABase/eabase.h(724): error C2371: 'char16_t' : redefinition; different basic types
1> C:\Program Files (x86)\Microsoft Visual Studio 11.0\VC\include\yvals.h(528) : see declaration of 'char16_t'

Visual studio 2012, x64 build.

@rparolin

This comment has been minimized.

Show comment
Hide comment
@rparolin

rparolin Feb 11, 2016

Member

I've reproduced this locally. I'll take a look.

Member

rparolin commented Feb 11, 2016

I've reproduced this locally. I'll take a look.

@c6burns

This comment has been minimized.

Show comment
Hide comment
@c6burns

c6burns Feb 11, 2016

This seems to stem from char16_t and char32_t being simply typedefs in VS 12.0 (thus affecting VS2012 and 2013). Therefore _HAS_CHAR16_T_LANGUAGE_SUPPORT is 0, but it is typedef'd and _CHAR16T is defined for us. Switching eabase.h like so resolved the issue for me in VS2013:

            #if (!defined(_HAS_CHAR16_T_LANGUAGE_SUPPORT) || !_HAS_CHAR16_T_LANGUAGE_SUPPORT) && !defined(_CHAR16T)
                typedef wchar_t  char16_t;
                typedef uint32_t char32_t;
            #endif

c6burns commented Feb 11, 2016

This seems to stem from char16_t and char32_t being simply typedefs in VS 12.0 (thus affecting VS2012 and 2013). Therefore _HAS_CHAR16_T_LANGUAGE_SUPPORT is 0, but it is typedef'd and _CHAR16T is defined for us. Switching eabase.h like so resolved the issue for me in VS2013:

            #if (!defined(_HAS_CHAR16_T_LANGUAGE_SUPPORT) || !_HAS_CHAR16_T_LANGUAGE_SUPPORT) && !defined(_CHAR16T)
                typedef wchar_t  char16_t;
                typedef uint32_t char32_t;
            #endif
@Ono-Sendai

This comment has been minimized.

Show comment
Hide comment
@Ono-Sendai

Ono-Sendai Feb 11, 2016

Another way to solve the problem is to use the same typedef as MS does in yvals.h:

typedef unsigned short char16_t

Ono-Sendai commented Feb 11, 2016

Another way to solve the problem is to use the same typedef as MS does in yvals.h:

typedef unsigned short char16_t

@danbolt

This comment has been minimized.

Show comment
Hide comment
@danbolt

danbolt Feb 12, 2016

This seems to be the same sort of build bug as issue #3.

danbolt commented Feb 12, 2016

This seems to be the same sort of build bug as issue #3.

@c6burns

This comment has been minimized.

Show comment
Hide comment
@c6burns

c6burns Feb 12, 2016

They "seem" the same, but they do not stem from exactly the same code and the resolution of the issues will likely not be the same

c6burns commented Feb 12, 2016

They "seem" the same, but they do not stem from exactly the same code and the resolution of the issues will likely not be the same

@Jasper-Bekkers

This comment has been minimized.

Show comment
Hide comment
@Jasper-Bekkers

Jasper-Bekkers Feb 12, 2016

@c6burns I made the following change locally:

            #if !defined(_HAS_CHAR16_T_LANGUAGE_SUPPORT) || !_HAS_CHAR16_T_LANGUAGE_SUPPORT && !defined(_CHAR16T)
                typedef wchar_t  char16_t;
                typedef uint32_t char32_t;
            #endif
            #if !defined(_CHAR16T)
                #define _CHAR16T
            #endif

Eg. I moved the #if !defined(_CHAR16T) below also, because otherwise the typedefs should just be omitted entirely. (since _CHAR16T would always be defined).

This seemed to be the only change (after fixing the cmake issue in #9 ) needed to build locally with VS2013.

Jasper-Bekkers commented Feb 12, 2016

@c6burns I made the following change locally:

            #if !defined(_HAS_CHAR16_T_LANGUAGE_SUPPORT) || !_HAS_CHAR16_T_LANGUAGE_SUPPORT && !defined(_CHAR16T)
                typedef wchar_t  char16_t;
                typedef uint32_t char32_t;
            #endif
            #if !defined(_CHAR16T)
                #define _CHAR16T
            #endif

Eg. I moved the #if !defined(_CHAR16T) below also, because otherwise the typedefs should just be omitted entirely. (since _CHAR16T would always be defined).

This seemed to be the only change (after fixing the cmake issue in #9 ) needed to build locally with VS2013.

@rparolin

This comment has been minimized.

Show comment
Hide comment
@rparolin

rparolin Feb 13, 2016

Member

Is this resolved? Just confirming before I close.

Member

rparolin commented Feb 13, 2016

Is this resolved? Just confirming before I close.

@Ono-Sendai

This comment has been minimized.

Show comment
Hide comment
@Ono-Sendai

Ono-Sendai Feb 13, 2016

I'm not sure, has anyone commited a fix?

Ono-Sendai commented Feb 13, 2016

I'm not sure, has anyone commited a fix?

@SneWs

This comment has been minimized.

Show comment
Hide comment
@SneWs

SneWs Feb 14, 2016

No fix has been committed from what I can see, and I can still repro the issue after a fresh sync using VS2013.

I would suggest that @Jasper-Bekkers creates a PR from his comment since it solves the issue.

Edit: VS2013, not 2015

SneWs commented Feb 14, 2016

No fix has been committed from what I can see, and I can still repro the issue after a fresh sync using VS2013.

I would suggest that @Jasper-Bekkers creates a PR from his comment since it solves the issue.

Edit: VS2013, not 2015

@c6burns

This comment has been minimized.

Show comment
Hide comment
@c6burns

c6burns Feb 14, 2016

OK PR is in ... I didn't put this in earlier because it breaks a number of the tests, but that can be handled as a separate issue

c6burns commented Feb 14, 2016

OK PR is in ... I didn't put this in earlier because it breaks a number of the tests, but that can be handled as a separate issue

@rparolin rparolin added bug and removed bug labels Feb 14, 2016

@c6burns

This comment has been minimized.

Show comment
Hide comment
@c6burns

c6burns Feb 15, 2016

OK investigating this library in more depth, I don't like this "fix" at all. Some additional changes to EABase are required to properly detect the compiler features and set appropriate defines, such as EA_WCHAR_UNIQUE and others. This will ensure we don't break 1/2 the string tests just to get the library to compile. In the case where people are actually using wchar strings, they will need all this working. This is my first experience with EABase - I'm used to a configure stage determining compiler features for me - but it's fairly logically laid out and commented so I'll take a look through and see what I can come up with

c6burns commented Feb 15, 2016

OK investigating this library in more depth, I don't like this "fix" at all. Some additional changes to EABase are required to properly detect the compiler features and set appropriate defines, such as EA_WCHAR_UNIQUE and others. This will ensure we don't break 1/2 the string tests just to get the library to compile. In the case where people are actually using wchar strings, they will need all this working. This is my first experience with EABase - I'm used to a configure stage determining compiler features for me - but it's fairly logically laid out and commented so I'll take a look through and see what I can come up with

@rparolin

This comment has been minimized.

Show comment
Hide comment
@rparolin

rparolin Feb 15, 2016

Member

0001-vs2012-fixes.zip
@c6burns Agreed. I actually have a fix for this coming, I wanted to run it against CI before submitting. Here is a patch file if you want to try it out as I'll be out for the rest of the night.

Member

rparolin commented Feb 15, 2016

0001-vs2012-fixes.zip
@c6burns Agreed. I actually have a fix for this coming, I wanted to run it against CI before submitting. Here is a patch file if you want to try it out as I'll be out for the rest of the night.

@c6burns

This comment has been minimized.

Show comment
Hide comment
@c6burns

c6burns Feb 15, 2016

One issue with the patch is you are using:
#if defined(EA_WHATEVER)
But EABase sets a lot of defines to 0, so you need:
#if defined(EA_WHATEVER) && EA_WHATEVER

This creates an issue in EASprintf.cpp where a section you intend to be excluded by the preprocessor is still being compiled. Otherwise, success :)

c6burns commented Feb 15, 2016

One issue with the patch is you are using:
#if defined(EA_WHATEVER)
But EABase sets a lot of defines to 0, so you need:
#if defined(EA_WHATEVER) && EA_WHATEVER

This creates an issue in EASprintf.cpp where a section you intend to be excluded by the preprocessor is still being compiled. Otherwise, success :)

@rparolin

This comment has been minimized.

Show comment
Hide comment
@rparolin

rparolin Feb 16, 2016

Member

Should be fixed now. :)

Member

rparolin commented Feb 16, 2016

Should be fixed now. :)

@rparolin rparolin closed this Feb 16, 2016

@SpAMCAN

This comment has been minimized.

Show comment
Hide comment
@SpAMCAN

SpAMCAN Jul 5, 2016

I seem to get this bug again in VS2013, exact same error, using a new pull from git (building as win32). I'm new to this, so not sure if it's just something I've set up incorrectly or a bug that's reared its head again.

1>d:\users\spam_can\repositories\tremor-engine\src\include\eabase\eabase.h(724): error C2371: 'char16_t' : redefinition; different basic types
1>          c:\program files (x86)\microsoft visual studio 12.0\vc\include\yvals.h(605) : see declaration of 'char16_t'

SpAMCAN commented Jul 5, 2016

I seem to get this bug again in VS2013, exact same error, using a new pull from git (building as win32). I'm new to this, so not sure if it's just something I've set up incorrectly or a bug that's reared its head again.

1>d:\users\spam_can\repositories\tremor-engine\src\include\eabase\eabase.h(724): error C2371: 'char16_t' : redefinition; different basic types
1>          c:\program files (x86)\microsoft visual studio 12.0\vc\include\yvals.h(605) : see declaration of 'char16_t'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment