Skip to content

i128 is being stabilized#235

Merged
alexcrichton merged 2 commits intorust-lang:masterfrom
mark-i-m:i128
Mar 19, 2018
Merged

i128 is being stabilized#235
alexcrichton merged 2 commits intorust-lang:masterfrom
mark-i-m:i128

Conversation

@mark-i-m
Copy link
Copy Markdown
Contributor

@mark-i-m mark-i-m commented Mar 18, 2018

@mark-i-m mark-i-m mentioned this pull request Mar 18, 2018
Merged
2 tasks
est31
est31 reviewed Mar 18, 2018
View reviewed changes
Comment thread src/lib.rs
#![feature(compiler_builtins)]
#![feature(core_intrinsics)]
#![feature(naked_functions)]
#![feature(i128_type)]
Copy link
Copy Markdown
Member

@est31 est31 Mar 18, 2018

Choose a reason for hiding this comment

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

You'd most likely have to do #[cfg_attr(stage0, feature(i128_type))] here, as on stage0 it is still needed.

Copy link
Copy Markdown
Contributor Author

@mark-i-m mark-i-m Mar 18, 2018

Choose a reason for hiding this comment

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

Hmm... It seemed to work fine when I made this change locally?

@alexcrichton
Copy link
Copy Markdown
Member

alexcrichton commented Mar 18, 2018

Thanks! I think it's ok if this is here, right? Does this fail the build?

@alexcrichton
Copy link
Copy Markdown
Member

alexcrichton commented Mar 18, 2018

(in that I'd prefer to land the rustc change first and then land the change here after that's gone through)

@PlasmaPower
Copy link
Copy Markdown

PlasmaPower commented Mar 18, 2018

It's sort of a deadlock. Rustc fails without this, but this fails without rustc.

@alexcrichton
Copy link
Copy Markdown
Member

alexcrichton commented Mar 18, 2018

The "failure" I think is deny(warnings), right? Can that be removed?

@PlasmaPower
Copy link
Copy Markdown

PlasmaPower commented Mar 18, 2018

Yeah I think that's the best solution. If we want we can re-add if after the rustc change goes through.

@mark-i-m
Copy link
Copy Markdown
Contributor Author

mark-i-m commented Mar 18, 2018

Moving the feature(i128_type) after allow(unused_features) seems to make the compiler build for me locally.

@kennytm
Copy link
Copy Markdown
Member

kennytm commented Mar 18, 2018

We could use the old external tool update workflow here:

  1. Push this PR into a new, non-master branch on this repository
  2. Update the submodule in E0263 needs to be updated to new format rust#35518 to point to that new branch
  3. Wait for E0263 needs to be updated to new format rust#35518 to merge and a new nightly is released
  4. Merge this PR

@PlasmaPower
Copy link
Copy Markdown

PlasmaPower commented Mar 18, 2018

That's what I originally suggested in the rustc PR but just ignoring the warning temporarily seems easier.

@mark-i-m
Copy link
Copy Markdown
Contributor Author

mark-i-m commented Mar 18, 2018

Umm... does anyone understand the current Travis failure:

thread 'rustc' panicked at 'slice index starts at 4294967295 but ends at 30759529', libcore/slice/mod.rs:791:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
note: rustc 1.26.0-nightly (adf2135ad 2018-03-17) running on x86_64-apple-darwin

@mark-i-m
Copy link
Copy Markdown
Contributor Author

mark-i-m commented Mar 18, 2018

It looks like the current change in the PR might be sufficient if we fix the build failures... alternately, I could do @kennytm's suggestion. Just let me know :)

@alexcrichton
Copy link
Copy Markdown
Member

alexcrichton commented Mar 19, 2018

Ok this strategy seems fine by me yeah

@alexcrichton alexcrichton merged commit 34f5d4a into rust-lang:master Mar 19, 2018
@mark-i-m
Copy link
Copy Markdown
Contributor Author

mark-i-m commented Mar 20, 2018

Hmm... this doesn't seem to have worked... I don't understand why.

PlasmaPower
PlasmaPower reviewed Mar 20, 2018
View reviewed changes
Comment thread src/lib.rs
#![feature(abi_unadjusted)]
#![feature(linkage)]
#![feature(lang_items)]
#![allow(unused_features)]
Copy link
Copy Markdown

@PlasmaPower PlasmaPower Mar 20, 2018

Choose a reason for hiding this comment

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

Oh this is #![allow(unused_features)], but you need #[allow(stable_features)].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@PlasmaPower PlasmaPower PlasmaPower left review comments

@est31 est31 est31 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mark-i-m @alexcrichton @PlasmaPower @kennytm @est31