[PATCH] D114415: [llvm] [Support] Add HTTP Server Support library.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/Support/HTTPServer.cpp:58
+          },
+          [=](bool Success) { delete MB; }};
+}
----------------
noajshu wrote:
> dblaikie wrote:
> > Could this parameter (the last lambda, the "CompletionHandler" member in StreamingHTTPResponse) be move-only (non-copyable) in that case it could capture-by-move the std::unique_ptr<MemoryBuffer> and be a bit more robust in terms of memory management?
> That's a great idea! Cpp-httplib converts the content provider to a [[ https://github.com/yhirose/cpp-httplib/blob/master/httplib.h#L336 | ContentProvider ]] which is a std::function. This ends up copying the lambda.
> 
> I was thinking we might refactor the interface, so instead of returning the response (or response provider) you can interact with a "HTTPServerRequest" object similar to how cpp-httplib's own API works.
> Their API is more flexible. For example, you can decide at runtime whether you want to do streaming response handling or return a single response string. In the wrapper implementation here you have to either return a StreamingHTTPResponse or an HTTPResponse, fixed at compile time.
> 
> Even if we mirror the httplib interface, the problem remains that we cannot pass a non-copyable lambda directly to httplib's `set_content_provider`. I'm not sure if there is a nice way around this problem.
Ah, oh well. Thanks for the context!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114415/new/

https://reviews.llvm.org/D114415