Skip to content

Conversation

@jrosdahl
Copy link
Contributor

In order to test the split version (.h + .cc via split.py):

  • Added a test_split program in the test directory whose main purpose is to verify that it works to compile and link the test case code against the split httplib.h version.
  • Moved types needed for test cases to the “header part” of httplib.h. Also added forward declarations of functions needed by test cases.

The changes to httplib.h just move code around (or add forward declarations), with one exception: detail::split and detail::process_client_socket have been converted to non-template functions (taking an std::function instead of using a type parameter for the function) and forward-declared instead. This avoids having to move the templates to the “header part”.

As discussed in #1008.

@yhirose
Copy link
Owner

yhirose commented Jul 31, 2021

@jrosdahl, thanks for the changes. They look good to me. I found a number of implementations of functions (like type Class::method(...) {) are missing inline keyword. Could you makes sure to put it to them?

In order to test the split version (.h + .cc via split.py):

- Added a test_split program in the test directory whose main purpose is
  to verify that it works to compile and link the test case code against
  the split httplib.h version.
- Moved types needed for test cases to the “header part” of httplib.h.
  Also added forward declarations of functions needed by test cases.
- Added an include_httplib.cc file which is linked together with test.cc
  to verify that inline keywords have not been forgotten.

The changes to httplib.h just move code around (or add forward
declarations), with one exception: detail::split and
detail::process_client_socket have been converted to non-template
functions (taking an std::function instead of using a type parameter for
the function) and forward-declared instead. This avoids having to move
the templates to the “header part”.
@jrosdahl jrosdahl force-pushed the test-split-httplib branch from 5e64db3 to a80b69c Compare July 31, 2021 13:10
@jrosdahl
Copy link
Contributor Author

Could you makes sure to put it to them?

Done.

@yhirose
Copy link
Owner

yhirose commented Jul 31, 2021

Everything now looks good. Thanks for your fine contribution!

@yhirose yhirose merged commit 887074e into yhirose:master Jul 31, 2021
@yhirose
Copy link
Owner

yhirose commented Jul 31, 2021

I particularly appreciated the idea to verify any inline keyword is missing with test/include_httplib.cc. :)

ExclusiveOrange pushed a commit to ExclusiveOrange/cpp-httplib-exor that referenced this pull request May 2, 2023
In order to test the split version (.h + .cc via split.py):

- Added a test_split program in the test directory whose main purpose is
  to verify that it works to compile and link the test case code against
  the split httplib.h version.
- Moved types needed for test cases to the “header part” of httplib.h.
  Also added forward declarations of functions needed by test cases.
- Added an include_httplib.cc file which is linked together with test.cc
  to verify that inline keywords have not been forgotten.

The changes to httplib.h just move code around (or add forward
declarations), with one exception: detail::split and
detail::process_client_socket have been converted to non-template
functions (taking an std::function instead of using a type parameter for
the function) and forward-declared instead. This avoids having to move
the templates to the “header part”.
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

Successfully merging this pull request may close these issues.

2 participants