electronicarts / EASTL

EASTL stands for Electronic Arts Standard Template Library. It is an extensive and robust implementation that has an emphasis on high performance.
BSD 3-Clause "New" or "Revised" License
7.82k stars 904 forks source link

Questionable EABase preprocessor expressions #97

Closed jspohr closed 7 years ago

jspohr commented 7 years ago

This regards EABase as of the 3.05.03 release.

When including eahave.h, VS2015 emits several warnings about evaluation of undefined preprocessor macros. I believe that these are minor bugs in the EABase headers:

eastl\test\packages\EABase\include\Common\EABase/config/eacompiler.h(275): warning C4668: 'EA_COMPILER_VERSION' is not defined as a preprocessor macro, replacing with '0' for '#if/#elif'

Code in question:

        #elif defined(_MSC_VER) && (EA_COMPILER_VERSION >= 1900)    // VS2015+ 
            #define EA_COMPILER_CPP14_ENABLED 1
        #endif

EA_COMPILER_VERSION is defined further down in eacompiler.h. So the expression will always yield false. _MSC_VER should probably be used instead of EA_COMPILER_VERSION, or the block should be moved.


eastl\test\packages\EABase\include\Common\EABase/eahave.h(483): warning C4668: '_HAS_CPP0X' is not defined as a preprocessor macro, replacing with '0' for '#if/#elif'

The same warning is also issued for lines 786, 799, 812, and 825. Code in question:

    #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) || _HAS_CPP0X) // Dinkumware. VS2010+
        #define EA_HAVE_CPP11_SYSTEM_ERROR 1

The last part of the expression triggers the warning. I'm not certain what the intention is here, but I believe that it may not be doing the right thing. The expression !(defined(_HAS_CPP0X) || _HAS_CPP0X) means that if _HAS_CPP0X is defined, we don't care about its value. If it's not defined however, it's value will always be false, so the right side of the expression is useless and could be removed. This might be OK, but maybe the expression was intended to do something else?

I can create a pull request for the first issue, as it's an easy fix, but I'm not certain about the second bunch of warnings, as I can't tell what the test should do exactly.

rparolin commented 7 years ago

The _HAS_CPP0X was a Dinkumware macro that enabled/disabled the TR1 additions. From my reading of the code this appears to just be a typo. It trying to disable specific headers and features not available under TR1.

https://connect.microsoft.com/VisualStudio/feedback/details/622227/vs2010-c-cli-linker-problem-duplicate-types-lnk2022-metadata-operation-failed

rparolin commented 7 years ago
--- //packages/EABase/dev/include/Common/EABase/config/eacompiler.h 2017-02-14 18:24:10.000000000 0100
+++ F:\packages\EABase\dev\include\Common\EABase\config\eacompiler.h    2017-02-14 18:24:10.000000000 0100
@@ -272,7 +272,7 @@
    #if !defined(EA_COMPILER_CPP14_ENABLED) && defined(__cplusplus)
        #if (__cplusplus >= 201402L)                                // Clang and GCC defines this like so in C++14 mode.
            #define EA_COMPILER_CPP14_ENABLED 1
-       #elif defined(_MSC_VER) && (EA_COMPILER_VERSION >= 1900)    // VS2015+ 
+       #elif defined(_MSC_VER) && (_MSC_VER >= 1900)   // VS2015+ 
            #define EA_COMPILER_CPP14_ENABLED 1
        #endif
    #endif
--- //packages/EABase/dev/include/Common/EABase/eahave.h    2017-02-14 18:24:10.000000000 0100
+++ F:\packages\EABase\dev\include\Common\EABase\eahave.h   2017-02-14 18:24:10.000000000 0100
@@ -480,7 +480,7 @@

 // #include <system_error> 
 #if !defined(EA_HAVE_CPP11_SYSTEM_ERROR) && !defined(EA_NO_HAVE_CPP11_SYSTEM_ERROR)
-   #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) || _HAS_CPP0X) // Dinkumware. VS2010+
+   #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) && _HAS_CPP0X) // Dinkumware. VS2010+
        #define EA_HAVE_CPP11_SYSTEM_ERROR 1
    #elif defined(EA_COMPILER_CPP11_ENABLED) && defined(EA_HAVE_LIBSTDCPP_LIBRARY) && defined(EA_COMPILER_CLANG) && (EA_COMPILER_VERSION >= 301) && !defined(EA_PLATFORM_APPLE)
        #define EA_HAVE_CPP11_SYSTEM_ERROR 1
@@ -783,7 +783,7 @@

 // <iterator>: std::begin, std::end, std::prev, std::next, std::move_iterator.
 #if !defined(EA_HAVE_CPP11_ITERATOR_IMPL) && !defined(EA_NO_HAVE_CPP11_ITERATOR_IMPL)
-   #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) || _HAS_CPP0X) // Dinkumware. VS2010+
+   #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) && _HAS_CPP0X) // Dinkumware. VS2010+
        #define EA_HAVE_CPP11_ITERATOR_IMPL 1
    #elif defined(EA_COMPILER_CPP11_ENABLED) && defined(EA_HAVE_LIBSTDCPP_LIBRARY) && defined(EA_COMPILER_GNUC) && (EA_COMPILER_VERSION >= 4006)
        #define EA_HAVE_CPP11_ITERATOR_IMPL 1
@@ -796,7 +796,7 @@

 // <memory>: std::weak_ptr, std::shared_ptr, std::unique_ptr, std::bad_weak_ptr, std::owner_less
 #if !defined(EA_HAVE_CPP11_SMART_POINTER_IMPL) && !defined(EA_NO_HAVE_CPP11_SMART_POINTER_IMPL)
-   #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) || _HAS_CPP0X) // Dinkumware. VS2010+
+   #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) && _HAS_CPP0X) // Dinkumware. VS2010+
        #define EA_HAVE_CPP11_SMART_POINTER_IMPL 1
    #elif defined(EA_COMPILER_CPP11_ENABLED) && defined(EA_HAVE_LIBSTDCPP_LIBRARY) && defined(EA_COMPILER_GNUC) && (EA_COMPILER_VERSION >= 4004)
        #define EA_HAVE_CPP11_SMART_POINTER_IMPL 1
@@ -809,7 +809,7 @@

 // <functional>: std::function, std::mem_fn, std::bad_function_call, std::is_bind_expression, std::is_placeholder, std::reference_wrapper, std::hash, std::bind, std::ref, std::cref.
 #if !defined(EA_HAVE_CPP11_FUNCTIONAL_IMPL) && !defined(EA_NO_HAVE_CPP11_FUNCTIONAL_IMPL)
-   #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) || _HAS_CPP0X) // Dinkumware. VS2010+
+   #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) && _HAS_CPP0X) // Dinkumware. VS2010+
        #define EA_HAVE_CPP11_FUNCTIONAL_IMPL 1
    #elif defined(EA_COMPILER_CPP11_ENABLED) && defined(EA_HAVE_LIBSTDCPP_LIBRARY) && defined(EA_COMPILER_GNUC) && (EA_COMPILER_VERSION >= 4004)
        #define EA_HAVE_CPP11_FUNCTIONAL_IMPL 1
@@ -822,7 +822,7 @@

 // <exception> std::current_exception, std::rethrow_exception, std::exception_ptr, std::make_exception_ptr
 #if !defined(EA_HAVE_CPP11_EXCEPTION_IMPL) && !defined(EA_NO_HAVE_CPP11_EXCEPTION_IMPL)
-   #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) || _HAS_CPP0X) // Dinkumware. VS2010+
+   #if defined(EA_HAVE_DINKUMWARE_CPP_LIBRARY) && (_CPPLIB_VER >= 520) && !(defined(_HAS_CPP0X) && _HAS_CPP0X) // Dinkumware. VS2010+
        #define EA_HAVE_CPP11_EXCEPTION_IMPL 1
    #elif defined(EA_COMPILER_CPP11_ENABLED) && defined(EA_HAVE_LIBSTDCPP_LIBRARY) && defined(EA_COMPILER_GNUC) && (EA_COMPILER_VERSION >= 4004)
        #define EA_HAVE_CPP11_EXCEPTION_IMPL 1
rparolin commented 7 years ago

Resolved with release 3.05.06.