Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upMake MSVC detection ludicrously robust #34492
Conversation
rust-highfive
assigned
alexcrichton
Jun 26, 2016
alexcrichton
reviewed
Jun 27, 2016
| } | ||
| } else { None } | ||
| } else { vec![] } |
This comment has been minimized.
This comment has been minimized.
alexcrichton
Jun 27, 2016
Member
Perhaps this could just be:
match (arch, host_arch()) {
// ...
}Can you also add docs as to what's being returned here? It's not clear to me at least what these are vectors of nor what each element of the pair is.
alexcrichton
reviewed
Jun 27, 2016
| }).and_then(|((mut cmd, host), sub)| { | ||
| get_ucrt_dir().map(|dir| { | ||
| debug!("Found Universal CRT {:?}", dir); | ||
| add_lib(&mut cmd, &dir.join("ucrt").join(sub)); |
This comment has been minimized.
This comment has been minimized.
alexcrichton
Jun 27, 2016
Member
This is pretty confusing using map and and_then purely for the side effect of calling add_lib, can this switch to using a macro or the original form of control flow?
retep998
force-pushed the
retep998:please-be-robust-already
branch
from
f460adc
to
40a7175
Jun 27, 2016
This comment has been minimized.
This comment has been minimized.
|
Updated Also added a commit with one function converted to use |
This comment has been minimized.
This comment has been minimized.
|
Yeah the logic has gotten complicated enough in a few places that I'd be fine using that macro for most of the functions, other than that looks good to me! |
retep998
force-pushed the
retep998:please-be-robust-already
branch
from
19ebd6b
to
570fd7a
Jun 28, 2016
This comment has been minimized.
This comment has been minimized.
|
I've converted what I could to using the macro. |
kennytm
reviewed
Jun 28, 2016
| Some(val) => val, | ||
| None => return None, | ||
| }) | ||
| } |
This comment has been minimized.
This comment has been minimized.
kennytm
Jun 28, 2016
Member
This macro has been written at least twice in librustc under different names
try_opt!:rust/src/librustc/dep_graph/dep_node.rs
Lines 13 to 20 in 382ab92
option_try!:rust/src/librustc/util/common.rs
Lines 72 to 74 in bb4b3fb
This comment has been minimized.
This comment has been minimized.
retep998
Jun 28, 2016
Author
Member
Clearly this just indicates the need for this sort of macro in libstd. Sure would be nice if the new ? stuff supported Option.
This comment has been minimized.
This comment has been minimized.
|
Tidy looks like it's failing on travis, but otherwise r=me |
retep998
force-pushed the
retep998:please-be-robust-already
branch
from
570fd7a
to
a1b33b4
Jun 28, 2016
This comment has been minimized.
This comment has been minimized.
|
Travis appears to have passed. |
This comment has been minimized.
This comment has been minimized.
|
@bors r=alexcrichton |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Jul 1, 2016
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@bors: retry
On Fri, Jul 1, 2016 at 5:38 AM, bors notifications@github.com wrote:
|
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Jul 1, 2016
This comment has been minimized.
This comment has been minimized.
|
|
retep998 commentedJun 26, 2016
Resurrection of #31158
r? @alexcrichton