Implementations of templated methods are missing when splitting into .h and .cc

[jrosdahl]

Thanks for writing cpp-httplib!

I made an attempt to use the non-header-only version of httplib (in ccache) to reduce the time to recompile code that uses the library. However, the templated implementation of e.g. set_connection_timeout ends up in httplib.cc and therefore results in a linker error since the needed template has not been instantiated.

It looks like these methods are affected:

template <typename T>
T Request::get_header_value(const char *key, size_t id = 0) const;

template <typename T>
T Response::get_header_value(const char *key, size_t id = 0) const;

template <typename... Args>
ssize_t Stream::write_format(const char *fmt, const Args &...args);

template <class Rep, class Period>
Server &Server::set_read_timeout(const std::chrono::duration<Rep, Period> &duration);

template <class Rep, class Period>
Server &set_write_timeout(const std::chrono::duration<Rep, Period> &duration);

template <class Rep, class Period>
Server &set_idle_interval(const std::chrono::duration<Rep, Period> &duration);

template <typename T>
T Result::get_request_header_value(const char *key, size_t id = 0) const;

template <class Rep, class Period>
void ClientImpl::set_connection_timeout(const std::chrono::duration<Rep, Period> &duration);

template <class Rep, class Period>
void ClientImpl::set_read_timeout(const std::chrono::duration<Rep, Period> &duration);

template <class Rep, class Period>
void ClientImpl::set_write_timeout(const std::chrono::duration<Rep, Period> &duration);

template <class Rep, class Period>
void Client::set_connection_timeout(const std::chrono::duration<Rep, Period> &duration);

template <class Rep, class Period>
void Client::set_read_timeout(const std::chrono::duration<Rep, Period> &duration);

template <class Rep, class Period>
void Client::set_write_timeout(const std::chrono::duration<Rep, Period> &duration);

Would you welcome a pull request that moves some or all of these into the header file part? If not, perhaps some caveat text could be added to the Split httplib.h into .h and .cc section?

Related: I tried to modify the test suite to run against a split version to make it possible to verify that the split version works, but I realized it will be impossible to do without moving the tests that depend on details into a separate test file. Is that something that would be of interest?

[yhirose]

@jrosdahl, thank you for the good feedback! I have implemented split.py at #287 requested by a user, but I have honestly never used it in my projects and the latest httplib.h doesn't work with split.py any more...

Could please you send a pull request including your change?

Related: I tried to modify the test suite to run against a split version to make it possible to verify that the split version works, but I realized it will be impossible to do without moving the tests that depend on details into a separate test file. Is that [something that would be of interest?

As long as split.py just splits httplib.h and doesn't do any modification, I think we don't need test the split version.

Thank you for your help!

[jrosdahl]

Could please you send a pull request including your change?

I've so far only experimented with fixing the methods I needed so I haven't tried a more complete solution, but I'll give it a try! The problem is verifying that it works. Which leads to...:

As long as split.py just splits httplib.h and doesn't do any modification, I think we don't need test the split version.

If there are no tests of the split version then I imagine there could be regression in the future again, right? That is, we may add a templated method into the .cc part that actually is needed in the .h part, just like the methods I listed. I'm kind of thinking the other way around: if we only check the split version we don't need to test the single-header version since we know that it has access to everything that the split version has.

I can try to refactor the tests as I envision them to see if it works out (if only to test my actual changes). Then you can make the call if you want to go with the restructured tests or not. (This is under the assumption that I manage to split them in a reasonable and not too time consuming way – haven't tried yet.) Sounds OK?

[yhirose]

Since I consider httplib.h the only source of truth and generated files with split.py are by-product to me, I would not like to test against the split version with test/Makefile. But I am ok to provide another Makefile and test source file in order to detect compile or link errors due to the misplacement of templated methods. How about adding test/Makefile.split and test/test_split.cc where you can do whatever things you would like to do? (There is already a similar make file test/Makefile.fuzz_test for fuzz testing.)

[jrosdahl]

Perfectly reasonable to keep httplib.h the only source of truth.

I've started looking into this now and it seems to me that there are two approaches I could take to get test coverage for the split case:

• Approach A:
  • Add implementations of templated API methods to the "header part" of httplib.h.
  • Also add forward declarations, types and template implementations of methods needed by tests to the "header part".
  • Keep tests as is, but run them once for the non-split case and once for the split case.
  • Pros: All tests are run for both the non-split and split cases. Developers don't have to think about where to place tests.
  • Cons: Developers have to think more about where to declare and implement stuff in httplib.h. Requires more duplication in form of forward declarations.
• Approach B:
  • Add implementations of templated API methods to the "header part" of httplib.h.
  • Split tests into two parts: one with tests that can be compiled with only access to the public API and one that requires access to detail. (A test that tests the public API may require helpers from detail and therefore needs to be in the latter test suite.)
  • Run "test_public" + "test_detail" with the non-split httplib.h and "test_public" with the split httplib.h.
  • Pros: Developers only have to put implementations of templated public API methods into the "header part" of httplib.h.
  • Cons: Many tests will not be run with the split version. Developers have to think about where to place new tests.

I personally would go with approach A, but I guess both approaches may be too intrusive or painful for you as a maintainer so it's entirely your call. 🙂

Anyway, I'm doing approach A to test my (future) changes. Let's see how it plays out.

[yhirose]

Thanks for the detail. I like the approach A as well.

Cons: Developers have to think more about where to declare and implement stuff in httplib.h. Requires more duplication in form of forward declarations.

I think the cost paid by the approach A is acceptable unless there are too many forward declarations.
Hope your work will go well and I am looking forward to the pull request. But no rush!