Skip to content

Make MSVC detection ludicrously robust#34492

Merged
bors merged 1 commit into
rust-lang:masterfrom
retep998:please-be-robust-already
Jul 2, 2016
Merged

Make MSVC detection ludicrously robust#34492
bors merged 1 commit into
rust-lang:masterfrom
retep998:please-be-robust-already

Conversation

@retep998
Copy link
Copy Markdown
Contributor

Resurrection of #31158

r? @alexcrichton

Comment thread src/librustc_trans/back/msvc/mod.rs Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@retep998 retep998 force-pushed the please-be-robust-already branch from f460adc to 40a7175 Compare June 27, 2016 18:18
@retep998
Copy link
Copy Markdown
Contributor Author

Updated bin_subdir and the comments for it.

Also added a commit with one function converted to use otry!. Can you tell me what you think and whether I should apply that style to the rest of the functions?

@alexcrichton
Copy link
Copy Markdown
Member

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 retep998 force-pushed the please-be-robust-already branch from 19ebd6b to 570fd7a Compare June 28, 2016 11:28
@retep998
Copy link
Copy Markdown
Contributor Author

I've converted what I could to using the macro.

Comment thread src/librustc_trans/back/msvc/mod.rs Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This macro has been written at least twice in librustc under different names 😆

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearly this just indicates the need for this sort of macro in libstd. Sure would be nice if the new ? stuff supported Option.

@alexcrichton
Copy link
Copy Markdown
Member

Tidy looks like it's failing on travis, but otherwise r=me

Should fix a few more edge cases

Fixes rust-lang#31151
Fixes rust-lang#32159
Fixes rust-lang#34484
Improves rust-lang-deprecated/rust-packaging#50

Signed-off-by: Peter Atashian <retep998@gmail.com>
@retep998 retep998 force-pushed the please-be-robust-already branch from 570fd7a to a1b33b4 Compare June 28, 2016 20:30
@retep998
Copy link
Copy Markdown
Contributor Author

Travis appears to have passed.

@sfackler
Copy link
Copy Markdown
Member

@bors r=alexcrichton

@bors
Copy link
Copy Markdown
Collaborator

bors commented Jun 29, 2016

📌 Commit a1b33b4 has been approved by alexcrichton

@bors
Copy link
Copy Markdown
Collaborator

bors commented Jul 1, 2016

⌛ Testing commit a1b33b4 with merge ed33659...

@bors
Copy link
Copy Markdown
Collaborator

bors commented Jul 1, 2016

💔 Test failed - auto-win-msvc-64-opt-rustbuild

@alexcrichton
Copy link
Copy Markdown
Member

@alexcrichton
Copy link
Copy Markdown
Member

@bors: retry

  • v

On Fri, Jul 1, 2016 at 5:38 AM, bors notifications@github.com wrote:

💔 Test failed - auto-win-msvc-64-opt-rustbuild
https://buildbot.rust-lang.org/builders/auto-win-msvc-64-opt-rustbuild/builds/1626


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#34492 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAD95Laha4Rqk-EAzk9d8w8-n1iLQqWrks5qRQoogaJpZM4I-q8o
.

@bors
Copy link
Copy Markdown
Collaborator

bors commented Jul 1, 2016

⌛ Testing commit a1b33b4 with merge 23095ae...

@bors
Copy link
Copy Markdown
Collaborator

bors commented Jul 1, 2016

💔 Test failed - auto-win-msvc-64-opt-rustbuild

@alexcrichton
Copy link
Copy Markdown
Member

@bors
Copy link
Copy Markdown
Collaborator

bors commented Jul 2, 2016

⌛ Testing commit a1b33b4 with merge 7e07e31...

bors added a commit that referenced this pull request Jul 2, 2016
Make MSVC detection ludicrously robust

Resurrection of #31158

r? @alexcrichton
@bors bors merged commit a1b33b4 into rust-lang:master Jul 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants