Replace shared_ptr with unique_ptr for better performance

[Snape3058]

According to the bug reports of my smart pointer analysis tool, you have a 'unique shared_ptr' problem. As the shared_ptr uses more resources than the unique_ptr, if the ownership is not semantically shared among multiple shared_ptrs, the unique_ptr is preferred for better performance.

Although my tool did not find all the problems, I tried my best to understand your code and fix all the appearances of the problem. This patch fixes the problem by replacing the related shared_ptr with unique_ptr and using std::make_unique and std::move to create and transfer the unique_ptr respectively. As the function std::make_unique was introduced in C++14, I also changed the C++ standard to C++14 in the first commit.

The problems are caused by Result::res_ and Client::cli_. As these two member variables only initialized by their corresponding constructors and they are not used as a shared_ptr in the class. Using unique_ptr instead of shared_ptr for managing these non-shared objects could be much better.

I have no ideas about whether the objects are required to be accessed through a getter method in the future. But, you can simply add a getter method returning a raw pointer with the unique_ptr::get() method, when the getters are needed and the ownership is not shared.

Here are the three bug reports generated by my smart pointer analysis tool.

• http://lcs.ios.ac.cn/~maxt/SPelton/reports/report-1ff7a1.html
• http://lcs.ios.ac.cn/~maxt/SPelton/reports/report-37e384.html
• http://lcs.ios.ac.cn/~maxt/SPelton/reports/report-9aa0d3.html

Sorry for renaming httplib.h as analyze.cpp during checking.

[yhirose]

@Snape3058, thanks for bringing it to our attention. The change looks good to me except using std::make_unique because it's a C++11 library. (In the future, I'll move to C++17, but it's not going to happen any time soon.)
So could you make the adjustment not to use std::make_unique? Thanks!

[Snape3058]

C++14 is removed, std::make_unique is backported if C++ standard is lower than C++14.

I split this patch into two commits, one for backporting and one for replacing. The replacing commit can also be applied to the cpp17 branch directly, as far as I am thinking.

Maybe you want the backported make_unique declared in another namespace other than std, please tell me the name you prefer. However, the second patch may not work for the cpp17 branch after renaming because of the namespace specifier. Or you can change the name in the future commits yourself.

[yhirose]

@Snape3058, thanks for the change. I reviewed the code.

Maybe you want the backported make_unique declared in another namespace other than std, please tell me the name you prefer.

I would like to use backport_cxx14 namespace directly (or detail is fine with me) for this shim. So actual usage becomes backport_cxx14::make_unique.

However, the second patch may not work for the cpp17 branch after renaming because of the namespace specifier. Or you can change the name in the future commits yourself.

In the cpp17 branch, you don't have to do anything because it's just an experimental branch.

I found two things when reviewing the code:

1. I am concerned with where the make_unique shim code comes from. Did you come up with the implementation from scratch, or copy the code from somewhere else? I am very much sensitive with any legal issues. So whenever I include code from other source even if the license is public domain, I put the reference like this:

cpp-httplib/httplib.h

Lines 1286 to 1288 in fffbf1a

	// NOTE: This code came up with the following stackoverflow post:
	// https://stackoverflow.com/questions/180947/base64-decode-snippet-in-c
	inline std::string base64_encode(const std::string &in) {

2. Using __cplusplus in Visual C++ is problematic even in the latest 2019. Users need to set /Zc:__cplusplus option in order to make __cplusplus work, but we can't force users to do so. So for the MSVC compiler, _MSC_VER should be used.
   https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/

[Snape3058]

I would like to use backport_cxx14 namespace directly (or detail is fine with me) for this shim. So actual usage becomes backport_cxx14::make_unique.

I prefer detail, as it is shorter. :-)

I am concerned with where the make_unique shim code comes from.

Thank you for reminding me about the problem. The original shim was copied from gcc's libstdc++, which may probably conflict with your MIT Lisence. Therefore, I replace it with another implementation from stack overflow, which is a widely used implementation in a lot of open source projects.

So for the MSVC compiler, _MSC_VER should be used.

According to my investigation, the macro _MSC_LANG defines the correct __cplusplus value on MSVC, i.e. if /Zc:__cplusplus is enabled, __cplusplus == _MSVC_LANG. Therefore, I uses this macro for MSVC and __cplusplus for other compilers. However, as I have little experience programming with MSVC, I do not know whether I have made it correct. If there is any problem or other things that I need to correct, please contact me.

[Snape3058]

Maybe testing the C++ standard is no longer required after changing std::backport_cxx14::make_unique to detail::make_unique.

The original purpose of testing C++ standard is to prevent the conflict of std::backport_cxx14::make_unique, which is also used as std::make_unique, with the std::make_unique in C++14. However, if detail::make_unique is used, there is no longer such a problem. I have removed the C++ standard test in the latest commit.

[yhirose]

@Snape3058, looks very good! Thanks for your contribution!