ProtoDef-io / protodefc

Compiler for protodef written in Rust
11 stars 3 forks source link

Implement standard IO for the compile command #8

Closed wtfaremyinitials closed 7 years ago

wtfaremyinitials commented 7 years ago

Enables command line use like so:

$ protodefc compile javascript - - < test.pds

However there are some memory safety concerns here.

From the FromRawFd docs

This function is also unsafe as the primitives currently returned have the contract that they are the sole owner of the file descriptor they are wrapping. Usage of this function could accidentally allow violating this contract which can cause memory unsafety in code that relies on it being true.

If standard input or output (like println!) are used anywhere else in the code, memory unsafety could arise.

wtfaremyinitials commented 7 years ago

I used from_raw_fd because the struct returned by std::io::stdin is Stdin, not File. I suppose Box<Write> could probably work, but it would force rustc to use dynamic dispatch for write calls.

hansihe commented 7 years ago

There is only a single read and write call. The overhead of using a trait object would be negligible in this case.

In this case, wouldn't it be better to have a read_input and write_output function which takes care of reading and writing? You wouldn't lose anything by doing this, as we are only really writing/reading a string anyways.

If we wanted things to be slightly more efficient in the future, we could make the backends take a T: Write and write directly to those. By doing that we would avoid having to write to a String, then writing that to stdout.

Sorry if I come off as overly negative here, but I like to avoid unsafe code as much as I can, and this is a case where it is relatively easily avoidable.

wtfaremyinitials commented 7 years ago

While on the topic of backend interface for output, would it be worthwhile to support backends generating multiple files? For example a C backend might want to generate a pair of .c and .h files.

hansihe commented 7 years ago

Definitely, but my view on this stuff is that we should take the most straightforward route until we need something more. I generally find it much easier to design a good solution for something if I am more familiar with what I need.

This might tie into the idea of having per-target configuration files.

hansihe commented 7 years ago

Thanks!