Building with Visual Studio?

emmenlau commented 5 years ago

Dear developers, I very much cherish the moderns C++ postgreSQL interface. However it seems currently a bit hard to user with MSVC. Are you interested in supporting this platform? From my experience it can be slightly tedious work, but often it improves the overall code quality. If this is relevant for you I could try to make an Azure DevOps CI setup that provides continuous builds for your Github repository?

d-frey commented 5 years ago

We already support Visual Studio. We have a CI build running with Visual Studio 2017, see https://ci.appveyor.com/project/taocpp/taopq

Can you clarify what is "hard to use with MSVC"? Or what else is missing?

emmenlau commented 5 years ago

Dear @d-frey , thanks for the very quick reply! It comes as a big surprise for me that VS2017 is supported, because the build fails for me with quite a number of errors. Maybe I'm just setting wrong build options?

I had first issues with:


D:\taopq\include\tao/pq/result_traits.hpp(24): error C2433: 'result_traits_size': 'inline' not permitted on data declarations
D:\taopq\include\tao/pq/result_traits.hpp(27): error C2433: 'tao::pq::result_traits_size<`template-type-parameter-1', ?? :: ?? >': 'inline' not permitted on data declarations
D:\taopq\include\tao/pq/result_traits.hpp(30): error C2433: 'result_traits_has_null': 'inline' not permitted on data declarations
D:\taopq\include\tao/pq/result_traits.hpp(33): error C2433: 'tao::pq::result_traits_has_null<`template-type-parameter-1', ?? >': 'inline' not permitted on data declarations

I quickly fixed these myself in result.hpp and result_traits.hpp by removing inline. But then the build fails again with:

D:\taopq\include\tao/pq/internal/pool.hpp(101): error C2039: 'weak_from_this': is not a member of 'tao::pq::internal::pool<tao::pq::connection>'
D:\taopq\include\tao/pq/connection_pool.hpp(19): note: see declaration of 'tao::pq::internal::pool<tao::pq::connection>'
D:\taopq\include\tao/pq/internal/pool.hpp(98): note: while compiling class template member function 'std::shared_ptr<tao::pq::connection> tao::pq::internal::pool<tao::pq::connection>::get(void)'
D:\taopq\include\tao/pq/connection_pool.hpp(43): note: see reference to function template instantiation 'std::shared_ptr<tao::pq::connection> tao::pq::internal::pool<tao::pq::connection>::get(void)' being compiled
D:\taopq\include\tao/pq/connection_pool.hpp(20): note: see reference to class template instantiation 'tao::pq::internal::pool<tao::pq::connection>' being compiled
D:\taopq\include\tao/pq/internal/pool.hpp(101): error C2660: 'tao::pq::internal::pool<tao::pq::connection>::attach': function does not take 1 arguments

So then I was under the assumption that MSVC is not supported. Do these errors mean anything to you? Is my VS2017 Service Pack too old, or am I doing something else wrong?

d-frey commented 5 years ago

I'm not a Windows-developer, so I can only guess. We do require C++17, which would explain the 'inline' not permitted on data declarations - it was an error before C++17.

Which build system are you using? The supplied CMakeLists.txt should set all the required switches, e.g. /std:c++17. You can see the resulting options here: https://ci.appveyor.com/project/taocpp/taopq/build/job/41g0j67yaojb70fy

ColinH commented 5 years ago

The other error looks like the same thing, weak_from_this() was added to enable_shared_from_this in C++17.

emmenlau commented 5 years ago

I'm not sure why this won't work for me. Here is one of the failing build commands, and as you can see there is -std:c++17 passed to the compiler:

FAILED: CMakeFiles/taopq.dir/src/lib/pq/table_writer.cpp.obj 
D:\vs2017\VC\Tools\MSVC\14.11.25503\bin\Hostx64\x64\cl.exe   /TP  -ID:\taopq\include -ID:\taopq\src -ID:\artifacts-MSVC-Generic-7-x64-cl19.11.25547\Debug\include -ID:\artifacts-MSVC-Generic-7-x64-cl19.11.25547\Debug\include\server /MDd /Zi  /DDEBUG /DWINVER=_WIN32_WINNT_WIN7 /D_WIN32_WINNT=_WIN32_WINNT_WIN7  /DWIN32 /D_WINDOWS /W3 /GR /EHsc /MDd /Zi /Ob0 /Od /RTC1   -std:c++17 /FoCMakeFiles\taopq.dir\src\lib\pq\table_writer.cpp.obj /FdCMakeFiles\taopq.dir\taopq.pdb /FS -c D:\taopq\src\lib\pq\table_writer.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.11.25547 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

D:\taopq\include\tao/pq/result_traits.hpp(24): error C2433: 'result_traits_size': 'inline' not permitted on data declarations
D:\taopq\include\tao/pq/result_traits.hpp(27): error C2433: 'tao::pq::result_traits_size<`template-type-parameter-1', ?? :: ?? >': 'inline' not permitted on data declarations
D:\taopq\include\tao/pq/result_traits.hpp(30): error C2433: 'result_traits_has_null': 'inline' not permitted on data declarations
D:\taopq\include\tao/pq/result_traits.hpp(33): error C2433: 'tao::pq::result_traits_has_null<`template-type-parameter-1', ?? >': 'inline' not permitted on data declarations
D:\taopq\include\tao/pq/result.hpp(35): error C2433: 'has_reserve': 'inline' not permitted on data declarations
D:\taopq\include\tao/pq/result.hpp(38): error C2433: 'tao::pq::internal::has_reserve<`template-type-parameter-1', ?? :: ?? >': 'inline' not permitted on data declarations

ColinH commented 5 years ago

And that does the same as /std:c++17 ?

emmenlau commented 5 years ago

Thanks for all your help! I can only guess, but there are many indicators that the spelling of the flag is not the problem:

According to unofficial sources on the internet, options can be passed with either - or /.
Also, this flag is generated by cmake and I could not find reports of problems with the current writing.
Last, not least, the Visual Studio compiler is not complaining about an unknown or unsupported flag. So it seems at least highly unlikely that the spelling with - is causing the problem.

As a last resort, I will update my Visual Studio 2017 from 15.4 to 15.9 (current latest) in the hope that there are relevant improvements in the c++17 standard. I will get back to you soon!

emmenlau commented 5 years ago

Thank you @ColinH, so I was finally able to solve the issue: Visual Studio 2017 15.4 is not able to build taopq right now, but 15.9 is! The build works now, I will start with the tests soon.

ColinH commented 5 years ago

Sorry to not have been of much help due to not currently using MSVC ... but glad to hear you got everything up and running! @d-frey Can we put this under "Status" together with minimum required GCC and Clang (and PostgreSQL) versions?

d-frey commented 5 years ago

I wish I'd know what version is the required minimum or what "version" even means in the context of MSVC. Look at this: https://en.wikipedia.org/wiki/Microsoft_Visual_C%2B%2B#Internal_version_numbering

15.4 is not even listed in the above link, and I can't figure out when weak_from_this was added to Visual Studio.

So what should we do? List 15.4 as "too old"? List 15.9 as "known to work"? Both is not what one would really like to do. I would like to add a real minimum version, but when I just put 15.9 there, I might drive away users who are stuck with 15.7 even though it would work.

emmenlau commented 5 years ago

@d-frey , I feel your pain. This is a part where MSVC really sucks. For some releases I could find detailed tables that list the added features. But these details are usually found on the devblog and not in the official release notes. Also they don't exist for every release.

I would proceed as you suggested, to say that "users reported 15.9 (and newer) to be working", whereas "15.4 was reported not to be working". Its better than nothing and by far less work than an elaborate solution.

If you want to go to that length, the best would be to implement actual checks in cmake that will report at configure-time if the compiler misses required features, like in this example. Even better would be if cmake supports all your required features so you could just request them and hopefully cmake does the rest?

d-frey commented 5 years ago

Sadly, compiler vendors are either lying, or they only mean "C++17, the language", not "C++17, the language and the standard library", or they simply forgot some small detail or they think they implemented it correctly but have bugs, ... it's really complicated and between Marketing pushing for a C++17 checkmark on the box, time-pressure, etc., I can't even blame them.

CMake feature-based checks are also not good enough (we tried it in the past) as they become tedious to maintain, the can not detect small details (like P0033R1 is implemented, but one function is still missing), they do not account for bugs, etc. etc. - we therefore simply went with "You need C++17, period."

I will put some better description in the docs, but in the end I guess it will never be prefect ;)

taocpp / taopq

Building with Visual Studio? #16