microsoft / vcpkg

C++ Library Manager for Windows, Linux, and MacOS
MIT License
22.8k stars 6.3k forks source link

[yaml-cpp] warnings when using dynamic library #4205

Open HarryDC opened 6 years ago

HarryDC commented 6 years ago

When using yaml-cpp built as a dynamic library under visual studio there are warning issued C4251 (needs to have dll interface) and C4275 (non-dll interface used as base for dll-interface). This seems to stem from an action done in the portfile

When building yaml-cpp the portfile patches the file dll.h lines 58-64

file(READ ${CURRENT_PACKAGES_DIR}/include/yaml-cpp/dll.h DLL_H)
if(VCPKG_LIBRARY_LINKAGE STREQUAL "dynamic")
    string(REPLACE "#ifdef YAML_CPP_DLL" "#if 1" DLL_H "${DLL_H}")
else()
    string(REPLACE "#ifdef YAML_CPP_DLL" "#if 0" DLL_H "${DLL_H}")
endif()
file(WRITE ${CURRENT_PACKAGES_DIR}/include/yaml-cpp/dll.h "${DLL_H}")

This probably doesn't matter on static builds but for dynamic builds this cause the dll.h file to define

#define YAML_CPP_API __declspec(dllexport)

unconditionally, which in turn causes visual studio to issue a warning when using the yaml-cpp file. This block was introduced last year. I am assuming to fix another issue but there isn't really any explanation in the log, i'd just remove the block from the portfile, but it's hard to know what might get broken. Additionally it doesn't look like YAML_CPP_DLL gets defined in the portfile on dynamic builds. That should probably be passed to vcpkg_configure_cmake

jeanga commented 6 years ago

It has also been reported into yaml-cpp but empty_scalar static member (in node_data.h) misses a YAML_CPP_API (to dllexport it). It is mandatory for data members to be explicitely exported.

yaml-cpp\node\detail\node_data.h(84):
static YAML_CPP_API std::string empty_scalar;

instead of: static std::string empty_scalar;

another, better change would be to declare this as a static member function returning a const ref to a static var: static const std::string& empty_scalar() { static const std::string _empty_scalar; return _empty_scalar; } (and you need to change the (few) references to empty_scalar to empty_scalar() I have tested it. It works but requires more changes.

HTH Jean

HarryDC commented 6 years ago

It seems that the warnings were left in knowingly (https://github.com/Microsoft/vcpkg/issues/1621)

JackBoosY commented 5 years ago

Hi @HarryDC , thanks for reporting this issue. I didn't see any problem with built yaml-cpp, and there was no error when I used it(except Waring C4251). So, my question is, how much does this problem affect you? If this problem is not fatal, I will close it. Otherwise, I will make a patch to solve it. Thanks.

jeanga commented 5 years ago

Problem is not fatal but I would really like to get rid of the noise to my compilation output. (esp. when the pages of warnings intermix with actual error messages) Sincerely, I am currently looking for altenatives to this library just because of the warnings annoyance. (I really like yaml-cpp. it is fast and easy to use but I like a clean compiler output better)

Cheney-W commented 4 years ago

@jeanga If you just want to have a clean compiler output, you can use this patch locally. 0004-disable-warning.zip