-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace all const evaluation with miri #46882
Conversation
7de6a56
to
ef9043b
Compare
src/librustc_mir/hair/cx/expr.rs
Outdated
value: cx.tcx().mk_const(ty::Const { | ||
val: if cx.tcx().sess.opts.debugging_opts.miri { | ||
let inst = ty::Instance::new(def_id, substs); | ||
let ptr = cx.tcx() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eddyb help! I'm not sure where to start touching the hair in order to get these 'gcx and 'tcx lifetimes satisfied:
error[E0623]: lifetime mismatch
--> src/librustc_mir/hair/cx/expr.rs:603:38
|
584 | fn method_callee<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
| ------------------ this parameter and the return type are declared with different lifetimes...
...
587 | -> Expr<'tcx> {
| ----------
...
603 | let ptr = cx.tcx()
| ^^^ ...but data from `cx` is returned here
error: aborting due to previous error
error: Could not compile `rustc_mir`.
☔ The latest upstream changes (presumably #47018) made this pull request unmergeable. Please resolve the merge conflicts. |
☔ The latest upstream changes (presumably #47013) made this pull request unmergeable. Please resolve the merge conflicts. |
Great work on this, @oli-obk! I was just playing around with it myself, and was wondering on the state of it. Specifically, I seem to still be getting errors indicating that if expressions and loops are still not supported. Is this the case? |
This PR does not enable much new stuff. That's not the point of it. We can enable more things after this PR. All I'm trying to do is to make it work on all code that worked before, without adding anything that we want to talk about in RFCs. That said, a few things will work after this PR.
Also I haven't pushed everything yet. |
@oli-obk Oh, I see. Thanks for the clarification. |
@oli-obk this is currently tagged as waiting-on-author, but did you want to start getting an early review? |
FWIW, @eddyb and I have been expanding on this significantly over the last few days. He's encouraging me to submit a PR, though I don't know if it's going to be against oli-obk's branch, or rust-lang:master. |
☔ The latest upstream changes (presumably #47276) made this pull request unmergeable. Please resolve the merge conflicts. |
src/librustc_trans/mir/constant.rs
Outdated
.expect("miri alloc not found"); | ||
Ok(global_initializer(ccx, alloc)) | ||
} | ||
|
||
impl<'a, 'tcx> MirContext<'a, 'tcx> { | ||
// Old version of trans_constant now used just for SIMD shuffle | ||
pub fn remove_me__shuffle_indices(&mut self, | ||
pub fn remove_me_shuffle_indices(&mut self, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOL this wasn't a typo, I meant it to be an ugly name like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad. I got an error for non-snake-case names, and fixed it by removing the _
instead of adding a deny
attribute. :-P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah if it triggered the lint then no worries! I didn't know it cared.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, it's picky.
Just wanted to note that 1. the PR description should include all changes 2. this will need a crater run. |
I am now getting
Which very likely results from the fact that Any clues how to solve this? |
You removed the I'm going to need access to the miri allocation interner to be able to hash AllocIds in a stable manner |
@oli-obk, yes, we are creating rust/src/librustc_driver/driver.rs Line 189 in 8ff449d
This could be solved by some refactoring, I think. @eddyb wanted to move HIR lowering to after the There might also be another way of hashing |
We could have an |
During lowering we won't have any Won't just hashing the ID be problematic if recompiling changes the value of a static? |
That depends. Can you access the contents of an allocation via an Is there a write up somewhere what these |
Nope no writeup. I'll add one though In short : AllocIds are used to identify interned Allocations. Serialization serializes the allocation instead of the allocid and deserialization generates a new allocid on the fly to keep them unique |
Is there one graph of allocations per |
🎆 Wooooo! Congratulations, everyone! |
Getting a weird build crash on FreeBSD after this merge #48888 |
(This was buggy before rust-lang#46882)
Correct a few stability attributes * `const_indexing` language feature was stabilized in 1.26.0 by #46882 * `Display` impls for `PanicInfo` and `Location` were stabilized in 1.26.0 by #47687 * `TrustedLen` is still unstable so its impls should be as well even though `RangeInclusive` was stabilized by #47813 * `!Send` and `!Sync` for `Args` and `ArgsOs` were stabilized in 1.26.0 by #48005 * `EscapeDefault` has been stable since 1.0.0 so should continue to show that even though it was moved to core in #48735 This could be backported to beta like #49612
I'm sorry if this isn't the right place, but are tuple struct constructors available in the latest stable build (2018-03-25)? |
This is as good a place as any other ;) No they're not in the current stable. they'll be in the next stable (so they are in the current beta) |
a
and read from fieldb
const X: &u32 = &22
)1i8 >> [8][0]
does not lint about exceeding bitshifts anymore.foo[I]
produces a const eval lint iffoo: [T; N]
andN < I
union
s to implementtransmute
forCopy
types in constants without a feature gate. With all the greatness and nasal demons that come with this.&'static T
in constants (useful for embedded)fixes #34997 (stack overflow with many constants)
fixes #25574 (deref byte strings in patterns)
fixes #27918 (broken mir ICE)
fixes #46114 (ICE on struct constructors in patterns)
fixes #37448 (
SomeStruct { foo } as SomeStruct
)fixes #43754 (
return
in const fn)fixes #41898 (tuple struct constructors)
fixes #31364 (infinite recursion with const fn, fixed by miri's recursion limit)
closes #29947 (const indexing stabilization)
fixes #45044 (pattern matching repeat expressions)
fixes #47971 (ICE on const fn + references)
fixes #48081 (ICE on cyclic assoc const error)
fixes #48746 (nonhelpful error message with unions)
r? @eddyb
even though 1k loc are added in tests, this PR reduces the loc in this repository by 700