Skip to content

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Dec 1, 2017

We bumped the minimum LLVM to 3.9 in #45326. This just cleans up the conditional code in the rustllvm C++ wrappers to assume that minimum, and similarly cleans up the rustc_llvm build script.

We bumped the minimum LLVM to 3.9 in rust-lang#45326.  This just cleans up the
conditional code in the rustllvm C++ wrappers to assume at least 3.9.
@rust-highfive
Copy link
Contributor

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -96,11 +83,11 @@ fn main() {
let version_output = output(&mut version_cmd);
let mut parts = version_output.split('.').take(2)
.filter_map(|s| s.parse::<u32>().ok());
let (major, minor) =
let (major, _minor) =
if let (Some(major), Some(minor)) = (parts.next(), parts.next()) {
(major, minor)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this panic! if it encounters a version < 3.9?

Copy link
Member Author

Choose a reason for hiding this comment

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

We certainly could, but this is also checked in bootstrap.

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 2, 2017
@hanna-kruppe
Copy link
Contributor

LGTM, nice cleanup!

Supposedly I have bors access now, let's try this: @bors r+

@bors
Copy link
Collaborator

bors commented Dec 2, 2017

@rkruppe: 🔑 Insufficient privileges: Not in reviewers

@bors
Copy link
Collaborator

bors commented Dec 2, 2017

@rkruppe: 🔑 Insufficient privileges: and not in try users

@hanna-kruppe
Copy link
Contributor

🤔 rust-lang/rust-central-station#26 🤔

@alexcrichton
Copy link
Member

@rkruppe oops sorry I forgot to restart rcs to load the new changes, you should be good to go now!

@bors: r=rkruppe

@bors
Copy link
Collaborator

bors commented Dec 2, 2017

📌 Commit 5c4452a has been approved by rkruppe

@kennytm kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 3, 2017
@bors
Copy link
Collaborator

bors commented Dec 3, 2017

⌛ Testing commit 5c4452a with merge 1956d55...

bors added a commit that referenced this pull request Dec 3, 2017
Assume at least LLVM 3.9 in rustllvm and rustc_llvm

We bumped the minimum LLVM to 3.9 in #45326.  This just cleans up the conditional code in the `rustllvm` C++ wrappers to assume that minimum, and similarly cleans up the `rustc_llvm` build script.
@bors
Copy link
Collaborator

bors commented Dec 3, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: rkruppe
Pushing 1956d55 to master...

@bors bors merged commit 5c4452a into rust-lang:master Dec 3, 2017
@cuviper cuviper deleted the min-llvm-3.9 branch January 26, 2018 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants