JeffGarland / liaw2019-process

Repository for initial drafting of boost.process standards paper
MIT License
5 stars 4 forks source link

[Stdio Design] Designates Initializers & Extensions #62

Open klemens-morgenstern opened 3 years ago

klemens-morgenstern commented 3 years ago

@JeffGarland Should ramblings like this go into the std proposal?

1 Designated initializers

We COULD use designated initializers like so:

template<typename In = FILE*, typename Out = FILE*, typename Err = FILE*>
struct process_io
{
    In  in  = stdin;
    Out out = stdout;
    Err err = stderr;
};

auto test() 
{
    return process_io{.in= stdin, .err = stderr};
}

This easens the syntax, but captures by copy. In boost::process everything is captured by reference, but conceptually, i actually don't dislike this. If we use this with a pipe, what you usually want to to assign one pipe end to the child, and then close it on the father. This would look like this:

auto [in, out] std::pstream{}:
auto init = std::process_io{.out = std::move(in)};

The downside is that a pstream is copyable (which clones the handle), so that the naive solution would lead to potential deadlocks:

auto [in, out] std::pstream{}:
auto init = std::process_io{.out = in};

This could be avoided by making pipes not-copyable, which might be the best solution, since fstream can't be copied either. There is however a difference between pipes & files, in that files may be reopened by their name, while there's no way to do so with a pipe. But, I don't see any application for this - plus, if we can get the native_handle from a pipe and can construct another pipe from the handle, you can easily implement this corner case on your own.

klemens-morgenstern commented 3 years ago

2 Concepts for process_io

In order to make process_io extensible, I would add a type trait that allows users to add their own types. I consider the extensibility a necessity, since the user might bring his own pipe or networking library, which he might want to assign to the standard I/O. E.g. a named pipe server on windows might be used to spawn a process on connect that handles the pipe I/O or an HTTP server might foward it's open TCP socket to a worker process. A popular example of using stdio for this kind of communication is sftp.

The exensibility is not an absolute necessity, since one could use the native-handle directly instead. The type-safe extensibility would avoid bugs though.

Since we are using C++20 already, I would not got for a trait with boolean flags, but (similar to std::hash) a class with two member functions the user can specify.

Functionally this is how it whould look:

template<typename T>
struct process_io_traits;

using native_stream_handle = int;

template<>
struct process_io_traits<FILE*> 
{
    static auto get_readable_handle(FILE* f) {return fileno(f);}
    static auto get_writeable_handle(FILE* f) {return fileno(f);}
};

template<typename T>
concept ProcessReadableStream = requires(T a) {
    { process_io_traits<std::remove_reference_t<T>>{}.get_readable_handle(a) } -> std::convertible_to<native_stream_handle >;
};

template<typename T>
concept ProcessWritableStream = requires(T a) {
    { process_io_traits<std::remove_reference_t<T>>{}.get_writeable_handle(a) } -> std::convertible_to<native_stream_handle >;
};

template<ProcessReadableStream In = FILE*, 
         ProcessWritableStream Out = FILE*, 
         ProcessWritableStream Err = FILE*>
struct process_io
{
    In  in  = stdin;
    Out out = stdout;
    Err err = stderr;
};

The get_readable_handle is allowed to throw exceptions if the value is invalid. There is however no guarantee that the value passed in is valid, meaning that a child can have invalid handles in it's stdio handles without the parent process knowing it.

It should however be specified as:

template<typename T>
struct process_io_traits;

template<typename In = FILE*, 
         typename Out = FILE*, 
         typename Err = FILE*>
   requires(
       requires(In in) { { process_io_traits<std::remove_reference_t<T>>{}.get_readable_handle(in) } -> std::convertible_to< /* implementation-defined */ native-stream-handle>;}
    && requires(Out out) { { process_io_traits<std::remove_reference_t<T>>{}.get_readable_handle(out) } -> std::convertible_to< /* implementation-defined */ native-stream-handle>;}
    && requires(Err err) { { process_io_traits<std::remove_reference_t<T>>{}.get_readable_handle(err) } -> std::convertible_to< /* implementation-defined */ native-stream-handle>;}
)
struct process_io
{
    In  in  = stdin;
    Out out = stdout;
    Err err = stderr;
};

In addition, process should have the following specializations:

template<>
struct process_io_traits</* implementation-defined */ native-stream-handle > {
    static auto get_readable_handle( native-stream-handle );
    static auto get_writeable_handle(native-stream-handle ); 
};

template<>
struct process_io_traits<FILE*> {
    static auto get_readable_handle(FILE* f);
    static auto get_writeable_handle(FILE* f); 
};

template<>
struct process_io_traits<pstream> {
    static auto get_readable_handle(pstream &);
    static auto get_writeable_handle(pstream &); 
};

template<>
struct process_io_traits<ipstream> {
    static auto get_readable_handle(pstream &);
};

template<>
struct process_io_traits<opstream> {
    static auto get_writeable_handle(pstream &);
};

template<>
struct process_io_traits<pipe> {
    static auto get_readable_handle(pipe&);
    static auto get_writeable_handle(pipe&); 
};

template<>
struct process_io_traits<pipe_read_end> {
    static auto get_readable_handle(pipe_read_end&);
};

template<>
struct process_io_traits<pipe_write_end> {
    static auto get_writeable_handle(pipe_write_end&);
};

template<>
struct process_io_traits<std::filesystem::path> {
    static auto get_readable_handle(std::filesystem::path&);
    static auto get_writeable_handle(std::filesystem::path&); 
};

template<> //could also be a new device `std::nulldev` / `std::cnull`
struct process_io_traits<nullptr_t> {
    static auto get_readable_handle(nullptr_t);
    static auto get_writeable_handle(nullptr_t); 
};

In addition the following could be specified if the internals of the different STL implementations allow it - it would however not be as useful, since we already have a specification for std::filesystem::path.

template<>
struct process_io_traits<fstream> {
    static auto get_readable_handle(fstream &);
    static auto get_writeable_handle(fstream &); 
};

template<>
struct process_io_traits<ifstream> {
    static auto get_readable_handle(ifstream&);
};

template<>
struct process_io_traits<ofstream> {
    static auto get_writeable_handle(ofstream&);
};

Furthermore the following would work with the networking TS:

template<>
struct process_io_traits<net::socket> {
    static auto get_readable_handle(net::socket &);
    static auto get_writeable_handle(net::socket &); 
};
klemens-morgenstern commented 3 years ago

Designated initializers don't work on MSVC, not sure if a implementation error or if gcc is to leniant. Relevant for #67.

klemens-morgenstern commented 3 years ago

Implemented in the the reference implementation for gcc.

JeffGarland commented 3 years ago

Two weeks ago I might have questioned if we could get FILE* past the committee bc it's using a C construct in a c++ interface. But the format library just had a positive reception for that from LEWG so it seems possible.

JeffGarland commented 3 years ago

Designated initializers don't work on MSVC, not sure if a implementation error or if gcc is to leniant. Relevant for #67.

Not sure -- is there a snippet we can put in godbolt to see what clang says?

klemens-morgenstern commented 3 years ago

The FILE* is really just there so we can reuse stdout, stderr and stdin. In my implementation I actuall use a special type as default, instead of FILE*.

namespace detail {

#if defined(__unix__)

using native_stream_handle = int;

struct default_stderr { static int get() {return STDERR_FILENO;} };
struct default_stdout { static int get() {return STDOUT_FILENO;} };
struct default_stdin  { static int get() {return STDIN_FILENO;} };

#else

using native_stream_handle = HANDLE;

struct default_stderr { static HANDLE get() {return GetStdHandle(STD_ERROR_HANDLE);} };
struct default_stdout { static HANDLE get() {return GetStdHandle(STD_OUTPUT_HANDLE);} };
struct default_stdin  { static HANDLE get() {return GetStdHandle(STD_INPUT_HANDLE);} };

#endif

}

template<typename In = detail::default_stdin, typename Out = detail::default_stdout, typename Err = detail::default_stderr>
requires(
       requires(In   in) { { process_io_traits<std::remove_reference_t<In >>::get_readable_handle(in) } -> std::convertible_to<detail::native_stream_handle>;}
    && requires(Out out) { { process_io_traits<std::remove_reference_t<Out>>::get_writable_handle(out)} -> std::convertible_to<detail::native_stream_handle>;}
    && requires(Err err) { { process_io_traits<std::remove_reference_t<Err>>::get_writable_handle(err)} -> std::convertible_to<detail::native_stream_handle>;}
)
struct process_io
{
    In   in = {};
    Out out = {};
    Err err = {};
//...

];
klemens-morgenstern commented 3 years ago

I wasn't able to reproduce the issue with a simple example, but I think this is semantically correct. A very much simplified example works on gce.