TODOs, notes and comments extracted from the paper

[eliaskosunen]

Oh man there's a lot of stuff to go through...

List of TODOs:

Wording:

• "Start of execution on process create" under "Design"
• "wait or join" under "Design"
• "on_success" and "on_setup" under "Design"

Code:

• Define concepts ProcessReadableStream and ProcessWritableStream
• "Clean" process::terminate(), process_group::terminate()

TODOs added by me (@eliaskosunen):

• Add rationale for not making process SemiRegular (not DefaultConstructible and Copyable)
• Add rationale for making the behavior of process::~process() inconsistent with std::thread

My (@eliaskosunen) comments:

• I don't think we should overload the name pid_t as defined in POSIX, because I think that'd be misleading on non-POSIX platforms. I've replaced it with pid_type. pid could also be an option.
• Does ProcessReadableStream mean the child can read from it, or that we can read from it? In other words, is it the child's stdin or stdout/stderr?
• Does pid_type satisfy TriviallyCopyable and/or Regular? The underlying type does satisfy both on POSIX and Windows, so it probably should.
• Should we change the name of environment to maybe process_environment or something similar? This would prevent collisions with P1275 and avoid using up a very useful name in std.
• What are the requirements for process::native_handle_type? TriviallyCopyable? StandardLayout? (Semi)Regular? Copyable?
• There are a number of functions taking an argument after a parameter pack. To my knowledge, this doesn't work: https://godbolt.org/z/ZiUUQG. Examples of this are one of the constructors of process and process_group::emplace. The parameter pack should always be the final parameter.
• Is process_group Copyable?
• Is environment::native_environment_type cheap to copy, since there's a constructor taking it by value?
• Should environment::get, set and reset have overloads taking params by rvalue reference?
• is ForwardRange the right choice for the return value of environment::keys and path, and environment::entry::as_vector? I think taking an OutputIterator or OutputRange and returning void or the past-the-end iterator would be a more idiomatic choice, and would allow the user to control allocations better.
• There are a number of functions taking a const string& (e.g. environment::find_executable, or environment::entry::entry). Would string_view be a better option?
• In "Enhanced system details", the system function is defined as taking a parameter pack. It should take a string. Also, in the function body, a non-existent variadic constructor of process is used.
• Should the "Extension of fstream" be added to basic_filebuf as well? I think it should.
• Should we consider having u8pipe, u16pipe and u32pipe in addition to pipe and wpipe (same applies for other types templated on character type). Maybe add SG16 to Audience?
• Why is the copy assignment of basic_pipebuf deleted, but copy constructor defaulted? This isn't consistent with basic_filebuf and basic_stringbuf.
• Should std::this_process::native_handle_type and pid_type be aliases of process::native_handle_type and process::pid_type, respectively? I can't see the reason to not do this.

Existing comments by others from the paper:

• @JeffGarland: In "Design" -> "Core language impact": "Bryce to provide information to Jeff"
• @JeffGarland: In "Header <process> synopsis": "I (Jeff G) basically lifted this out of boost.process and updated"
• @klemens-morgenstern: In process_io::process_io: "Additionally, a path should be possible to open a file just for the child process"
• @JeffGarland: In "Open Questions -> Can we piggyback on the thread's forward progress stuff for process as well? Can we assume all threads on the system behave like C++ threads?": "Seems doubtful -- network TS"
• @klemens-morgenstern: In "Open Questions -> environment and command line": "std::this_process::environment though. That would make a few things more obvious, because we have an environment class too, that shuold just be used to set it up for the subprocess."

Comments from Isabella Muerte regarding std::environment and https://wg21.link/p1275 etc:

I'd recommend you look into why the Rust community decided to deprecate things like home(), shell(), etc. due to various CVE related issues on some operating systems.
Also, the find_executable function is typically called which because it can find non-executables that can be run via the OS execute process machinery, such as a script or custom registered file type such as windows.

One thing I've considered is having std::environment be the type and then a std::env static inline instance.

[JeffGarland]

Design: I've emailed Bryce. For now I've moved to open questions basically as a 'help needed from committee' This 2 go along with that

Can we piggyback on the thread's forward progress stuff for process as well?
Can we assume all threads on the system behave like C++ threads?

[JeffGarland]

"Start of execution on process create" under "Design"

seemed overcome by events -- so removed

[JeffGarland]

I don't think we should overload the name pid_t as defined in POSIX

I checked this off because I agree and you fixed it.

[JeffGarland]

What are the requirements for process::native_handle_type? TriviallyCopyable? StandardLayout? (Semi)Regular? Copyable?

Given this interface:
using native_handle_type = implementation-defined;

native_handle_type native_handle() const;

I'd say clearly copyable and assignable. Our expectation is it's cheap to copy as well. But I believe the implementation is free to do what it pleases given implementation defined. My take is that for this paper it's 'good enough' - I'll let you agree by checking off the issue :)

[klemens-morgenstern]

Add rationale for not making process SemiRegular (not DefaultConstructible and Copyable)

It is default-constructible. It can't be copied since a process cannot be cloned.

[ ] Define concepts ProcessReadableStream and ProcessWritableStream

This is indeed a problem abd I don't know how to solve that in an elegant way.
Maybe just allow native_handles and then use overloads and not a concept. If someone wants to add a new type he can just subclass std::process_io

[] Add rationale for making the behavior of process::~process() inconsistent with std::thread

I am pretty certain, `thread() would terminate the thread if that was possible accross different OS system. But this is wildly inconsistent, hence the destructor goes nuclear.

[ ] "Clean" process::terminate(), process_group::terminate()

In case a graceful shutdown is referenced by that, this is because it's not possible on windows or at least too different. Same as with a std::thread::terminate.

[ ] Does ProcessReadableStream mean the child can read from it, or that we can read from it? In other words, is it the child's stdin or stdout/stderr?

Always named from the parent's perspective. So readable stream is something the child writes to it, i.e. stdout/stderr.

[ ] Does pid_type satisfy TriviallyCopyable and/or Regular? The underlying type does satisfy both on POSIX and Windows, (it's an int). I don't know if we should specify this.

• [ ] Should we change the name of environment to maybe process_environment or something similar? This would prevent collisions with P1275 and avoid using up a very useful name in std.
I thought about that, but both are doing the same thing. Well what P1275 has a std:: enviroment is std::this_process:: environment.

I don't think environments make sense without a process, P1275 references the current one. I actually think this can be misleading if we have `

[ ] There are a number of functions taking an argument after a parameter pack. To my knowledge, this doesn't work: https://godbolt.org/z/ZiUUQG. Examples of this are one of the constructors of process and process_group::emplace. The parameter pack should always be the final parameter.

It works with concepts though. I am somewhat sure that's standard behaviour, not only gcc.

[ ] Is process_group Copyable?
No

[ ] Is environment::native_environment_type cheap to copy, since there's a constructor taking it by value?

It's a pointer to an array, so yes.

[ ] Should environment::get, set and reset have overloads taking params by rvalue reference?

I don't think so, since any the value will either just be read or (as with set) will need to be copied bytewise anyhow, since it is reformatted.

[ ] is ForwardRange the right choice for the return value of environment::keys and path, and environment::entry::as_vector? I think taking an OutputIterator or OutputRange and returning void or the past-the-end iterator would be a more idiomatic choice, and would allow the user to control allocations better.

That's not a bad idea, since you could operate on a view here. It could be Bidirectional though I think.

[ ] There are a number of functions taking a const string& (e.g. environment::find_executable, or environment::entry::entry). Would string_view be a better option?

Yes.

[ ] Should the "Extension of fstream" be added to basic_filebuf as well? I think it should.

Yes.

[ ] Should we consider having u8pipe, u16pipe and u32pipe in addition to pipe and wpipe (same applies for other types templated on character type). Maybe add SG16 to Audience?

Possibly but this is so trivial to add, I wouldn't bother now. It depends on whether or not we change it according to my recent PR, where pipe is the class using the networking ts, which was previously async_pipe. THen it would leave us with the streams though. I think we can easily add this in the last phase if need be.

[ ] Should std::this_process::native_handle_type and pid_type be aliases of process::native_handle_type and process::pid_type, respectively? I can't see the reason to not do this.

Yes, but maybe specify them both as aliases of the same OS-type (i.e. int and void*)

I'd recommend why the Rust community decided to deprecate things like home(), shell(), etc. due to various CVE related issues on some operating systems.
Also, the find_executable function is typically called which because it can find non-executables that can be run via the OS execute process machinery, such as a script or custom registered file type such as windows.

One thing I've considered is having std::environment be the type and then a std::env static inline instance.

I don't get the CVE issues, when we already expose the whole environment.

[JeffGarland]

Define concepts ProcessReadableStream and ProcessWritableStream

For now, I put that into the process_cuts.org file. We can address this detail in a future proposal

[JeffGarland]

Should the "Extension of fstream" be added to basic_filebuf as well? I think it should.

Also solved by removal