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

Building with Visual Studio? #16

Closed
emmenlau opened this issue Jun 16, 2019 · 12 comments
Closed

Building with Visual Studio? #16

emmenlau opened this issue Jun 16, 2019 · 12 comments
Assignees
Labels

Comments

@emmenlau
Copy link
Contributor

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 d-frey self-assigned this Jun 16, 2019
@d-frey
Copy link
Member

d-frey commented Jun 16, 2019

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
Copy link
Contributor Author

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
Copy link
Member

d-frey commented Jun 16, 2019

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
Copy link
Member

ColinH commented Jun 16, 2019

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

@emmenlau
Copy link
Contributor Author

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
Copy link
Member

ColinH commented Jun 17, 2019

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

@emmenlau
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

ColinH commented Jun 24, 2019

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
Copy link
Member

d-frey commented Jun 24, 2019

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
Copy link
Contributor Author

@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
Copy link
Member

d-frey commented Jun 25, 2019

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 ;)

d-frey added a commit that referenced this issue Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants