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

YARP_CLEAN_API flag is currently broken #978

Closed
drdanz opened this issue Nov 29, 2016 · 7 comments
Closed

YARP_CLEAN_API flag is currently broken #978

drdanz opened this issue Nov 29, 2016 · 7 comments

Comments

@drdanz
Copy link
Member

drdanz commented Nov 29, 2016

If YARP_CLEAN_API is enabled and YARP_COMPILE_TESTS is disabled, YARP build currently fails with this error:

[ 25%] Building CXX object src/libYARP_OS/CMakeFiles/YARP_OS.dir/src/Module.cpp.o
In file included from <yarp>/src/libYARP_OS/include/yarp/os/impl/Logger.h:12:0,
                 from <yarp>/src/libYARP_OS/src/Module.cpp:13:
<yarp>/build/generated_include/yarp/conf/api.h:60:36: error: expected identifier before ‘__attribute__’
 #    define YARP_HELPER_DLL_EXPORT __attribute__ ((visibility("default")))
                                    ^
<yarp>/build/generated_include/yarp/conf/api.h:75:23: note: in expansion of macro ‘YARP_HELPER_DLL_EXPORT’
 #  define YARP_EXPORT YARP_HELPER_DLL_EXPORT
                       ^~~~~~~~~~~~~~~~~~~~~~
<yarp>/src/libYARP_OS/include/yarp/os/api.h:13:25: note: in expansion of macro ‘YARP_EXPORT’
 #    define YARP_OS_API YARP_EXPORT
                         ^~~~~~~~~~~
<yarp>/src/libYARP_OS/include/yarp/os/api.h:23:52: note: in expansion of macro ‘YARP_OS_API’
 #    define YARP_OS_DEPRECATED_API YARP_DEPRECATED YARP_OS_API
                                                    ^~~~~~~~~~~
<yarp>/src/libYARP_OS/include/yarp/os/Module.h:45:7: note: in expansion of macro ‘YARP_OS_DEPRECATED_API’
 class YARP_OS_DEPRECATED_API yarp::os::Module : public IConfig {
       ^~~~~~~~~~~~~~~~~~~~~~
In file included from <yarp>/src/libYARP_OS/src/Module.cpp:15:0:
<yarp>/src/libYARP_OS/include/yarp/os/Module.h:45:47: error: expected initializer before ‘:’ token
 class YARP_OS_DEPRECATED_API yarp::os::Module : public IConfig {
                                               ^
src/libYARP_OS/CMakeFiles/YARP_OS.dir/build.make:3254: recipe for target 'src/libYARP_OS/CMakeFiles/YARP_OS.dir/src/Module.cpp.o' failed
make[2]: *** [src/libYARP_OS/CMakeFiles/YARP_OS.dir/src/Module.cpp.o] Error 1
CMakeFiles/Makefile2:360: recipe for target 'src/libYARP_OS/CMakeFiles/YARP_OS.dir/all' failed
make[1]: *** [src/libYARP_OS/CMakeFiles/YARP_OS.dir/all] Error 2
Makefile:149: recipe for target 'all' failed
make: *** [all] Error 2

This is not currently tested on travis, and it is normally not used because tests cannot currently compile with that flag enabled, therefore if you enable tests (on by default) this flag is not used even if you enable it manually.

We should fix it, enable the build on travis and, at some point, fix the tests so that you can compile them with this flag enabled

@drdanz
Copy link
Member Author

drdanz commented Nov 29, 2016

After some more testing, I believe this is an issue with recent gcc versions (tested with 6.2.0). I don't see this with clang 3.8.1, and I cannot reproduce this on travis

@drdanz drdanz added Platform: Linux Affects: YARP v2.3.68 This is a known issue affecting YARP v2.3.68 Target: YARP v2.3.68.1 labels Nov 29, 2016
@drdanz
Copy link
Member Author

drdanz commented Nov 29, 2016

Also it does not fail C++11 is enabled

@drdanz
Copy link
Member Author

drdanz commented Nov 29, 2016

Adding -std=c++98 to the build flags fixes this issue. The problem comes from g++ defining (even if c++11/14 is not enabled) __cplusplus=201402L and therefore the [[deprecated]] construct is used but it is not recognized by gcc itself.

I'd say this is a bug in the compiler.

@traversaro
Copy link
Member

Gcc 6 switched the default c++ standard used to C++14 (https://gcc.gnu.org/gcc-6/changes.html) so the __cplusplus value make sense, the strange thing is that is not recognizing [[deprecated]].

@drdanz
Copy link
Member Author

drdanz commented Nov 30, 2016

The following code builds with -std=c++98 and -std=c++11 but not with -std=c++14:

#if defined(__clang__)
#  if ((__clang_major__ * 100) + __clang_minor__) >= 304 && __cplusplus > 201103L
#    define YARP_COMPILER_CXX_ATTRIBUTE_DEPRECATED 1
#  else
#    define YARP_COMPILER_CXX_ATTRIBUTE_DEPRECATED 0
#  endif
#elif defined(__GNUC__)
#  if (__GNUC__ * 100 + __GNUC_MINOR__) >= 409 && __cplusplus > 201103L
#    define YARP_COMPILER_CXX_ATTRIBUTE_DEPRECATED 1
#  else
#    define YARP_COMPILER_CXX_ATTRIBUTE_DEPRECATED 0
#  endif
#endif

#  ifndef YARP_DEPRECATED
#    if YARP_COMPILER_CXX_ATTRIBUTE_DEPRECATED
#      define YARP_DEPRECATED [[deprecated]]
#      define YARP_DEPRECATED_MSG(MSG) [[deprecated(MSG)]]
#    elif YARP_COMPILER_IS_GNU || YARP_COMPILER_IS_Clang
#      define YARP_DEPRECATED __attribute__((__deprecated__))
#      define YARP_DEPRECATED_MSG(MSG) __attribute__((__deprecated__(MSG)))
#    elif YARP_COMPILER_IS_MSVC
#      define YARP_DEPRECATED __declspec(deprecated)
#      define YARP_DEPRECATED_MSG(MSG) __declspec(deprecated(MSG))
#    else
#      define YARP_DEPRECATED
#      define YARP_DEPRECATED_MSG(MSG)
#    endif
#  endif

class __attribute__ ((visibility ("default"))) YARP_DEPRECATED foo {};
int main(){}

This also fails:

class YARP_DEPRECATED __attribute__ ((visibility ("default"))) foo {};

Both these work:

class YARP_DEPRECATED foo {};
class __attribute__ ((visibility ("default"))) foo {};

So it is the combination of the attributes that causes issues.

Clang does not have problems building this with -std=c++14

@drdanz
Copy link
Member Author

drdanz commented Nov 30, 2016

using [[gnu::visibility("default")]] instead of __attribute__ ((visibility ("default"))) seems to solve the problem

drdanz added a commit to drdanz/yarp that referenced this issue Nov 30, 2016
Since gcc 6.1, c++14 is enabled by default.
For some reason (maybe a bug in the compiler) this does not compile with
gcc and c++14 enabled (it works with clang though):

  class __attribute__ ((visibility ("default"))) [[deprecated]] foo {};

Fixes: robotology#978
@drdanz
Copy link
Member Author

drdanz commented Nov 30, 2016

Unfortunately [[gnu::visibility("default")]] solves this issues for classes, but not for functions that ignore c++11 style attributes printing a warning: attribute ignored [-Wattributes] warning and leaving out the function from the libraries.

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

2 participants