Added in error checking for the compile() and expand() functions.

[marco9999]

Initial attempt to resolve issue #192.

New functions try_compile() and try_expand() have been introduced that returns a result. I have introduced the Error struct type, which is used within the result in the form of Result<_, Error>. The error contains the type of error (enum ErrorKind, unused for now) and a detailed message (String) to display to the user.

I also had to change the signature of the pubic function get_compiler(), in order to propagate errors (in this specific case, it is because of environment vars missing) - if you don't want to do this, maybe I can put in try_get_compiler and get_compiler?

This was pretty fun to work on, I might have a go at some others. One problem I ran into was you can't propagate a Result<> from a closure (used within Option<>::unwrap_or_else() calls), so I moved the relevant code out into a match statement instead. Not sure if there was another way to do this, but I at least understand why it's not allowed.

[marco9999]

Hmm, I don't know how to properly handle error checking within a parallel loop... (didn't have the parallel feature turned on for myself).

I can think of one way, to have a shared vector that can hold the results (behind a mutex), then check for any errors in that, otherwise return an Ok.

[alexcrichton]

Thanks so much for the PR, things are looking great!

• if you don't want to do this, maybe I can put in try_get_compiler and get_compiler?

Yeah let's leave the signature the same and add a new function.

so I moved the relevant code out into a match statement instead

No worries! I find myself having to do that frequently as well.

Hmm, I don't know how to properly handle error checking within a parallel loop...

Yeah let's just collect the results into a vector and then inspect them after parallel finishes?

[alexcrichton]

Thanks so much for the PR, things are looking great!

• if you don't want to do this, maybe I can put in try_get_compiler and get_compiler?

Yeah let's leave the signature the same and add a new function.

so I moved the relevant code out into a match statement instead

No worries! I find myself having to do that frequently as well.

Hmm, I don't know how to properly handle error checking within a parallel loop...

Yeah let's just collect the results into a vector and then inspect them after parallel finishes?

[alexcrichton]

I think the original source of the error here is io::Error, so perhaps this could continue to return a io::Result?

[marco9999]

This function calls get_target() and try_get_compiler(), in which at least get_target() tries to determine if an environment variable exists. The std::env::var() function it calls doesn't return any kind of io::Error (it used to panic if there the result was None), so I don't think its possible to use this?

[alexcrichton]

Ah yes indeed, good point!

[alexcrichton]

We typically try to keep the fields of a struct private for maximal forwards compatibility, so could the ErrorKind enum and the fields of this struct be left private? We could always publicly expose them later, however!

[dtolnay]

👍 for private for now. Specifically #48 is asking for the content of stderr to be available in the error, which would need to be added here later.

[marco9999]

Sure. What happens when someone tries to use this outside of the crate though? As in, will there be some sort of visibility issue when using the Result<_, Error> from one of the try functions?

[alexcrichton]

Oh so for now you just won't be able to look inside of Error outside this crate. Eventually we can add accessors for various fields and such in a backwards compatible fashion, but for now external users will just see that an Error type exists.

[alexcrichton]

I think it'd be possible to cut down on the verbosity here by adding:

impl From<io::Error> for Error {
    // ...
}

and that way you can just use ?!

[marco9999]

Sure, I will take a look into this.

[alexcrichton]

This might be a little neater as a helper function, so you could do something like:

dst.join(obj.file_name().ok_or_else(|| err("failed to get details"))?);

(or something like that)

[marco9999]

Sure, I will take a look into this.

[marco9999]

Are you still supporting rustc 1.9.0?

[alexcrichton]

Everything looks great to me, thanks so much @marco9999! Want to update the Travis config to inrease the minimum version to a Rust with the ? operator? Othert than that I believe this is ready to merge.

[marco9999]

Updated the rust version to 1.13.0. I think everything is ready...

[alexcrichton]

Alright merged and good to go, thanks again @marco9999!