Move the CPPAD_LIB_EXPORT decorator from function definition to function declaration

[GiulioRomualdi]

This PR fixes #60

cc @traversaro @S-Dafarra

[CLAassistant]

All committers have signed the CLA.

[bradbell]

As I understand the code in define.hpp, if one is compiling on windows they should add

-D cppad_lib_EXPORTS=YES

to the cmake command line. Is this what you are doing ?

Also, I think that the CPPAD_LIB_EXPORT marco is also needed for the functions in the following include files:

1. cppad/example/code_gen_fun.hpp:
2. cppad/local/graph/cpp_graph_op.hpp:
3. cppad/local/graph/json_lexer.hpp:

Does this seem correct to you ?

[traversaro]

to the cmake command line. Is this what you are doing ?

No, the cppad_lib_EXPORTS is automatically defined by CMake if the cppad_lib target is compiled as a shared library, see https://cmake.org/cmake/help/latest/prop_tgt/DEFINE_SYMBOL.html .

[bradbell]

In that case, it does not need to be discussed in the CppAD the cmake documentation
https://coin-or.github.io/CppAD/doc/cmake.htm
because it is automatic. Thanks.

[traversaro]

Exactly.

[bradbell]

I must have done something wrong. I am testing the current version using the instructions under Visual Studio on
https://coin-or.github.io/CppAD/doc/cmake.htm#CMake%20Command.Visual%20Studio

and am getting the following errros:

LINK Pass 1: command "C:\PROGRA~2\MICROS~2\2019\COMMUN~1\VC\Tools\MSVC\1428~1.293\bin\Hostx86\x86\link.exe /nologo @CMakeFiles\example_json.dir\objects1.rsp /out:example_json.exe /implib:example_json.lib /pdb:C:\msys64\home\bradl\repo\cppad.git\build\example\json\example_json.pdb /version:0.0 /machine:X86 /debug /INCREMENTAL /subsystem:console -LIBPATH:C:\msys64\home\bradl\repo\cppad.git\build\cppad_lib ..\..\cppad_lib\cppad_lib.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTFILE:CMakeFiles\example_json.dir/intermediate.manifest CMakeFiles\example_json.dir/manifest.res" failed (exit code 1120) with the following output:
unary_op.cpp.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) void __cdecl CppAD::local::graph::json_parser(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const &,class CppAD::cpp_graph &)" (__imp_?json_parser@graph@local@CppAD@@YAXABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@AAVcpp_graph@3@@Z)
... snip ... 

[traversaro]

cppad_lib.lib was compiled as static library. Looking into https://github.com/coin-or/CppAD/blob/master/cppad_lib/CMakeLists.txt#L52, it seems so. In that case, it seems that the code in

CppAD/include/cppad/local/define.hpp

Line 60 in 79e1cfd

\def CPPAD_LIB_EXPORT

is not completely correct, as if the library was not compiled as shared, then CPPAD_LIB_EXPORT should be defined as empty and not as __declspec(dllimport) .
This is the reason why https://cmake.org/cmake/help/latest/module/GenerateExportHeader.html is tipically used to generate those headers, to generate different code depending on the type of library that is compiled (if shared or static).
However, if you only support consuming the library using CMake on Windows, a relatively easy fix is to modify the snippet of code as:

# if defined(_MSC_VER) && defined(cppad_lib_STATIC)
# ifdef  cppad_lib_EXPORTS
# define CPPAD_LIB_EXPORT __declspec(dllexport)
# else
# define CPPAD_LIB_EXPORT __declspec(dllimport)
# endif  // cppad_lib_EXPORTS
# else   // _MSC_VER
# define CPPAD_LIB_EXPORT
# endif

and then in CMake add:

get_target_property(cppad_lib_target_type cppad_lib TYPE)
if(cppad_lib_target_type STREQUAL "STATIC_LIBRARY")
  target_compile_definitions(cppad_lib PUBLIC cppad_lib_STATIC)
endif()

[traversaro]

Sorry for not noticing this before.

[traversaro]

An alternative to support shared libraries on Windows without manually managing the visibility with CPPAD_LIB_EXPORT is simply to use the CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS, that support compilation as a shared library exporting all symbols (expect for static variables).

[bradbell]

Thanks !!
I think this is confused by the fact that I was using static libraries for WIndows (when I could not get the dynamic onse to work). See the following code from cppad_lib/CMakeLists.txt:

IF( ${CMAKE_SYSTEM_NAME} STREQUAL "Windows" )
    MESSAGE( STATUS "Windows system so building static cppad_lib")
    ADD_LIBRARY( cppad_lib STATIC ${source_list} )
ELSE( ${CMAKE_SYSTEM_NAME} STREQUAL "Windows" )
    MESSAGE( STATUS "Not Windows system so building shared cppad_lib")
    ADD_LIBRARY( cppad_lib SHARED ${source_list} )
    SET_TARGET_PROPERTIES( cppad_lib PROPERTIES SOVERSION ${soversion} )
    #
    FIND_LIBRARY(dl_LIBRARY dl)
    IF( dl_LIBRARY )
        TARGET_LINK_LIBRARIES(cppad_lib ${dl_LIBRARY})
    ENDIF( dl_LIBRARY )
ENDIF( ${CMAKE_SYSTEM_NAME} STREQUAL "Windows" )

In fact, my cmake output has -- Windows system so building static cppad_lib

[traversaro]

Yes, for how the CMake is now, it is only possible to compile static libraries on Windows.

[bradbell]

I changed cppad_lib/CMakeLists.txt to build shared libraries on windows and fixed a few more problems that came up during nmake check; see 92c9946.

Now I am stuck on the following error during the nmake check command:

[ 33%] Linking CXX executable example_graph.exe
[ 33%] Built target example_graph
Scanning dependencies of target check_example_graph
NMAKE : fatal error U1077: '.\example_graph.exe' : return code '0xc0000135'
Stop.
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\bin\HostX86\x86\nmake.exe"' : return code '0x2'

You can build just the offending object with the command nmake check_example_graph.

You can also see the failure here:
https://ci.appveyor.com/project/bradbell/cppad/builds/37730701

[bradbell]

I think I figured it out. One needs to add the directory where the library was built to the system path.
I did this and the build went past that error but stopped at another point that has nothing to do with the dll.

[traversaro]

I think I figured it out. One needs to add the directory where the library was built to the system path.
I did this and the build went past that error but stopped at another point that has nothing to do with the dll.

A common strategy for that is to set:

set(CMAKE_RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin")
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/lib")
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/lib")

so that all the .exe and .dll during build go in build/bin , and so they are in the same directory automatically, without needing to tweak the system path. See https://github.com/robotology/how-to-export-cpp-library/blob/master/CMakeLists.txt#L41 for reference.

[bradbell]

Given the results of my testing, it seems to me that a static variable in a C++ template function can different values in the DLL than in the rest of the code for a program. Is this true ?

[traversaro]

Given the results of my testing, it seems to me that a static variable in a C++ template function can different values in the DLL than in the rest of the code for a program. Is this true ?

I am not sure what you mean by "can different values" ?

[bradbell]

Suppose I have a template function:

template <class Type>
Type  count_calls(void)
{   static Type count_ = Type(0);
    return( ++count_ );
}

On unix, there would be one counter for calls to count_calls both inside and outside the shared library. It seems to me there would be two counters, one for calls inside the DLL and one for calls outside the DLL on windows ?

Actually, I think what is causing the CppAD tests to fail is that there are two versions of the static function

static thread_alloc_info* thread_info(
        size_t             thread          ,
        bool               clear = false   )
    {   static thread_alloc_info* all_info[CPPAD_MAX_NUM_THREADS];
... snip ...

in the file include/cppad/utility/thread_alloc.hpp. One that gets used inside the DLL and another that gets used outisde the DLL.

More generallly, CppAD assumes there is one version of each function (that has a static variable). Care is take to make sure that these variables are different for different threads or they are only changed in single thread mode.

[bradbell]

I have reverted back to building a static version of the library when using windows; see the heading Visual Studio on
file:///home/bradbell/repo/cppad.git/doc/cmake.htm#CMake%20Command.Visual%20Studio

If someone wants to try and make the shared version work; they will need to re-enable the definition of CPPAD_LIB_EXPORT in define.hpp and CMakeLists.tst; see the changes to CMakeLists.txt and define.hpp on.
d4780b6

I started a discussion about this topic; see
#97

[Wang-Peng-debug]

Does anyone find the way of How to build a DLL file for CppAD on Windows? I saw you guys started a discussion for this but still found no ways of that, please tell me how if anyone knows this, which would be of much help for me and I would appreciated it so much!!!!!!!!!!!!!!!