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

Doesn't build with c++17 (VS 2017 15.5) #523

Closed
VikingExplorer opened this issue Aug 17, 2017 · 26 comments
Closed

Doesn't build with c++17 (VS 2017 15.5) #523

VikingExplorer opened this issue Aug 17, 2017 · 26 comments

Comments

@VikingExplorer
Copy link

Update 3 is causing compiler errors in CppRestSdk v 2.8

2>C:\Dev\Couloir\3pLibs\cpprestsdk-master\Release\include\cpprest/streams.h(900): error C2187: syntax error: 'identifier' was unexpected here
2>C:\Dev\Couloir\3pLibs\cpprestsdk-master\Release\include\cpprest/streams.h(1133): note: see reference to class template instantiation 'Concurrency::streams::basic_istream<_CharType>' being compiled
2>C:\Dev\Couloir\3pLibs\cpprestsdk-master\Release\include\cpprest/http_msg.h(553): error C2039: 'to_utf16string': is not a member of '`global namespace''
2>C:\Dev\Couloir\3pLibs\cpprestsdk-master\Release\include\cpprest/http_msg.h(553): error C3861: 'to_utf16string': identifier not found

					return buffer.bumpc().then([](

#ifndef _WIN32 // Required by GCC
typename
#endif
900 --.> concurrency::streams::char_traits::int_type) { return false; });

Compilation: c++14 /permissive- /Zc:throwingNew /Zc:inline /bigobj

Is there a simple code change I can make?

@VikingExplorer
Copy link
Author

Just verified that this problem occurs as well with the latest version. (which I think is 2.9?)

@VikingExplorer
Copy link
Author

Fixed 1st issue by going with gcc solution:

//#ifndef _WIN32 // Required by GCC
typename
//#endif
concurrency::streams::char_traits::int_type) { return false; });

@VikingExplorer
Copy link
Author

2nd issue:

1>C:\Dev\Couloir\3pLibs\Casablanca\RestSdk29\Release\include\cpprest/http_msg.h(571): error C2039: 'to_utf16string': is not a member of '`global namespace'' (compiling source file WebService.cpp)
1>C:\Dev\Couloir\3pLibs\Casablanca\RestSdk29\Release\include\cpprest/http_msg.h(571): error C3861: 'to_utf16string': identifier not found (compiling source file WebService.cpp)

571: void set_body(const utf16string &body_text, utf16string content_type = ::utility::conversions::to_utf16string("text/plain"))
or
571: void set_body(const utf16string &body_text, utf16string content_type = utility::conversions::to_utf16string("text/plain"))

generates the same error.

Changed to:
void set_body(const utf16string &body_text, utf16string content_type = L"text/plain")

This seemed to fix the 2nd error. Not sure if it's correct.

@VikingExplorer
Copy link
Author

Wow, this library is so out of wack. Very hard to use. I don't want spend a month figuring it out again. Now, I'm looking at:

6>cpprest141_2_9.lib(http_helpers.obj) : error LNK2019: unresolved external symbol deflate referenced in function "public: class std::vector<unsigned char,class std::allocator > __cdecl web::http::details::compression::stream_compressor::stream_compressor_impl::compress(unsigned char const *,unsigned __int64,bool)" (?compress@stream_compressor_impl@stream_compressor@compression@details@http@web@@qeaa?AV?$vector@EV?$allocator@E@std@@@std@@PEBE_K_N@Z)

The library is way out of date, and hasn't received any attention since 2015. Issues have been accumulating for two years.

@cmazakas
Copy link

Yeah, it's pretty much dead I guess. MS decided it wasn't profitable, I suppose, and it seems like resources are no longer being allocated towards it.

You're in luck though. Beast (low-level HTTP C++ lib) has been accepted into Boost. So now you can go ahead and start using Beast to code up your HTTP framework as you need to.

@VikingExplorer
Copy link
Author

VikingExplorer commented Aug 22, 2017

I didn't see it in Boost 1.65.

Here it is: https://github.com/boostorg/beast

@cmazakas
Copy link

Oh, my apologies. I should've specified that it's not in it quite yet but it's on the track of being included. Most likely in 1.66. I think it's also available in vcpkg as well.

@sinall
Copy link

sinall commented Aug 23, 2017

Good to know there is a better alternative.

@VikingExplorer
Copy link
Author

I agree. CppRestSdk was part of our strategy to eliminate our dependency on Poco++, which is huge, clumsy and ugly, when compared to Casablanca.

However, when Casablanca is compared to Beast, it appears to be big and ugly. As is typical of MS, they have a strong tendency to reject industry standards and traditions. For example, their obsession with Ugly & Stupid Nuget.

CppRestSdk is at a higher level of abstraction than Beast, so it doesn't look I'll be able to simply replace. I'll have to write some abstraction of the Beast interface. In the meantime, I've gone back to v2.8, using the code changes above.

@cmazakas
Copy link

cpprest is indeed significantly higher level.

Beast gives you the tools to create something like the cpprest sdk. It's based on ASIO so you kind of setup ASIO tcp stuff and then Beast helps you abstract away the request/response handling.

I've been using the Beast sample servers for my projects and you can see how usable it is. Maybe not for designing a full-on public-facing server but it's enough for service-oriented architectures.

@VikingExplorer
Copy link
Author

VikingExplorer commented Sep 2, 2017

When all else fails, read the documentation. Problem is, it still didn't work, but it took a really long time. Here is what I did:

1. https://github.com/Microsoft/vcpkg: download zip & extract 
2. C:\...\vcpkg> bootstrap-vcpkg.bat
    3. C:\...\vcpkg> vcpkg install cpprestsdk:x64-windows-static

This resulted in output libraries like: boost_system-vc140-mt-1_65.lib

Extremely disturbing that it created vc140, when everything I'm doing is using vc141. I've tried really hard to figure out the cluster-F$%k that is vcpkg / CMake in order to:

a) control the tool chain to be 141
b) control CRT linkage to make sure it's dynamic

Failing that, I believe that it wasn't able to automatically link boost.

I manually added the appropriate boost libs to the Linker input, but I suspect that (b) is incorrect, because I got

11>CouloirAFX-WinW.lib(LogSink.obj) : error LNK2001: unresolved external symbol "public: static class std::basic_string<wchar_t,struct std::char_traits<wchar_t>,class std::allocator<wchar_t> > const web::http::methods::GET" (?GET@methods@http@web@@2v?$basic_string@_WU?$char_traits@_W@std@@v?$allocator@_W@2@@std@@b)
11>CouloirAFX-WinW.lib(LogSink.obj) : error LNK2001: unresolved external symbol "public: static class std::basic_string<wchar_t,struct std::char_traits<wchar_t>,class std::allocator<wchar_t> > const web::http::methods::POST" (?POST@methods@http@web@@2v?$basic_string@_WU?$char_traits@_W@std@@v?$allocator@_W@2@@std@@b)
11>CouloirAFX-WinW.lib(LogSink.obj) : error LNK2001: unresolved external symbol "public: static class std::basic_string<wchar_t,struct std::char_traits<wchar_t>,class std::allocator<wchar_t> > const web::http::methods::PUT" (?PUT@methods@http@web@@2v?$basic_string@_WU?$char_traits@_W@std@@v?$allocator@_W@2@@std@@b)

vcpkg created cpprest_2_9.lib in both the debug and release builds. I love the fact that it has the same name, without any annoying "d". However, a debug/release confusion would be bad.

@VikingExplorer
Copy link
Author

In the triplets folder, I created: x64-windows-static-md.cmake

set(VCPKG_TARGET_ARCHITECTURE x64)
set(VCPKG_CRT_LINKAGE dynamic)
set(VCPKG_LIBRARY_LINKAGE static)
set(VCPKG_PLATFORM_TOOLSET v141)

In a VS2017 Cmd Prompt: vcpkg install cpprestsdk:x64-windows-static-md

In the folder: vcpkg\installed\x64-windows-static-md\debug\lib

Files:
boost_atomic-vc140-mt-gd-1_65.lib
boost_chrono-vc140-mt-gd-1_65.lib

As you can see, they are neither 141, nor are they md.

@ras0219-msft
Copy link
Contributor

Hi @VikingExplorer,

In order to force CMake to be able to find the boost lib files (it uses a small set of hardcoded names ☹️), Vcpkg always renames the files to look like static, vc140, mt. However, these files are built exactly how you specified: VS2017 compiler with dynamic CRT linkage.

I used the triplet you've posted and built a simple cmake file that uses boost:

>cmake .. -DCMAKE_TOOLCHAIN_FILE=... -DVCPKG_TARGET_TRIPLET=x64-windows-static-md -A x64
>cmake --build .
>./Debug/main.exe
1.2.11="D:\src\cmake-test\buildx64mt"

to prove that these binaries are not MT-linked, this project does indeed fail when building with the normal x64-windows-static triplet:

>cmake .. -DCMAKE_TOOLCHAIN_FILE=... -DVCPKG_TARGET_TRIPLET=x64-windows-static -A x64
>cmake --build .
error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MTd_StaticDebug' doesn't match value 'MDd_DynamicDebug'

This would repro the same inside Visual Studio, however you'll need to override the VcpkgTriplet MSBuild property to select your new triplet.


As for the first issue you posted; have you tried simply vcpkg install cpprestsdk?

@VikingExplorer
Copy link
Author

VikingExplorer commented Sep 3, 2017

Robert, for some weird reason, yesterday, I tried manually linking the boost libs and they did result in successful EXE links. That gave me the idea that the problem is really about libs being misnamed. Thank you for confirming that suspicion.

So, what can I change to get the Libs named correctly? This is important because the pragma statements are causing it to look for the correctly named libs.

@VikingExplorer
Copy link
Author

VikingExplorer commented Sep 5, 2017

Robert, I'm sorry for not being clear. I eventually broke down and used vcpkg. However, this did not (and could never) fix the first issue I identified. These are problems with cpprestsdk header files not compiling with c++17 permissive-. They definitely should.

@bandrews-spirent
Copy link

So what is the best way to build/run? This has been a huge PITA.
It doesn't build out of the box with VS2017. Even after NuGet package restore, it can't find openssl/conf.h
I did manage to build it successfully after hacking up the project file so that it can find everything it needs to build, but I have no confidence in running.
Running the BingRequest sample yields a crash trying to do a double free on http_request
Any help is much appreciated.

@VikingExplorer
Copy link
Author

VikingExplorer commented Sep 15, 2017 via email

@bandrews-spirent
Copy link

Thanks. I don't know much about vcpkg. Will I be able to customize the boost, ssl, etc. versions it uses? My application uses newer versions of those libraries, so I need to link to those.

@VikingExplorer
Copy link
Author

@bandrews-spirent hehe. Funny you should ask...

Nope, controversial, but vcpkg is hard coded to specific versions of various libraries. MS argument is that this is the only way to ensure that all the libraries are compatible with each other. That said, the version of Boost is the latest. Here is a list of libraries:

https://blogs.msdn.microsoft.com/vcblog/2016/09/19/vcpkg-a-tool-to-acquire-and-build-c-open-source-libraries-on-windows/

Here is issue I raised over at vcpkg, although hopefully the problem doesn't apply to you. It has the steps I went through to build cpprest (which depends on boost):

vcpkg issue

Good luck!

@VikingExplorer VikingExplorer changed the title VS 2017 update 3 Doesn't build with c++17 permissive- (VS 2017 update 3) Sep 16, 2017
@VikingExplorer
Copy link
Author

This issue will remain open until the underlying issue is fixed:

It's not about building cpprestsdk itself, but about building projects that use cpprestsdk, and are using c++17 permissive-

@bandrews-spirent
Copy link

@VikingExplorer Thanks for your response. Yeah, I don't think hard coding specific versions of libraries works too well in the real world. :) In a big application, you may have multiple dependencies on boost, and obviously you can't have multiple versions of a library linked into the same application. I guess this kind of complex dependency is another plus for micro services, but in my case pulling out the new cpprest dependency into another service (another process) is not feasible.
Anyways... I did get it to build and run. The heap corruption problem was a result of my own stupidity. I realized I was trying to build my app against different versions of the header files than the lib version I was linking to. Yikes!

@VikingExplorer
Copy link
Author

I was a bit scattered brain on this, so let me clarify the "issue". It's still happening with VS2017 15.5 c++17 cpprest_2_10.

Error C3083 '`global namespace'': the symbol to the left of a '::' must be a type (compiling source C:\Dev\Couloir\3pLibs\vcpkg\installed\x64\include\cpprest\http_msg.h 571

To fix it, I've been changing cpprest source code like this:

FROM: void set_body(const utf16string &body_text, utf16string content_type = ::utility::conversions::to_utf16string("text/plain"))

TO: void set_body(const utf16string &body_text, utf16string content_type = L"text/plain")

@VikingExplorer VikingExplorer changed the title Doesn't build with c++17 permissive- (VS 2017 update 3) Doesn't build with c++17 (VS 2017 15.5) Dec 9, 2017
@VikingExplorer
Copy link
Author

Error C7510 'int_type': use of dependent type name must be prefixed with 'typename' (compiling C:\Dev\Couloir\3pLibs\vcpkg\installed\x64\include\cpprest\streams.h 890

FROM: #ifndef _WIN32 // Required by GCC

TO: //#ifndef _WIN32 // Required by GCC & MSVC

@ras0219-msft
Copy link
Contributor

We did notice some of these issues and they should hopefully all be solved in master. Could you check and give us a second opinion?

If all is well, I'll look at making a new release soon.

@georgiosd
Copy link

I found that the int_type error goes away if you disable SDL checks

@VikingExplorer
Copy link
Author

6 months is too long to wait. Fixed by switching to boost::beast

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants