-
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
Deduplicate some code between miri and const prop #64419
Changes from all commits
bc17936
86c7c4d
ecc4cc2
1c219bb
644d4f3
11eb91f
9ec928c
c0c8ce8
fadfd92
15d2b7a
9333514
4e58e2e
dcc6c28
a99f255
ba2d6c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ use rustc::ty::{self, Ty, TyCtxt}; | |
use super::{ | ||
Allocation, AllocId, InterpResult, Scalar, AllocationExtra, | ||
InterpCx, PlaceTy, OpTy, ImmTy, MemoryKind, Pointer, Memory, | ||
Frame, Operand, | ||
}; | ||
|
||
/// Whether this kind of memory is allowed to leak | ||
|
@@ -184,6 +185,22 @@ pub trait Machine<'mir, 'tcx>: Sized { | |
dest: PlaceTy<'tcx, Self::PointerTag>, | ||
) -> InterpResult<'tcx>; | ||
|
||
/// Called to read the specified `local` from the `frame`. | ||
fn access_local( | ||
_ecx: &InterpCx<'mir, 'tcx, Self>, | ||
frame: &Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>, | ||
local: mir::Local, | ||
) -> InterpResult<'tcx, Operand<Self::PointerTag>> { | ||
frame.locals[local].access() | ||
} | ||
|
||
/// Called before a `StaticKind::Static` value is accessed. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please specify what "access" means here. Is this called when the address of a static is taken? Or just on reads/writes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will double check and improve the comment in the other open PR I have but I believe this is just on reads/writes. |
||
fn before_access_static( | ||
_allocation: &Allocation, | ||
) -> InterpResult<'tcx> { | ||
Ok(()) | ||
} | ||
|
||
/// Called to initialize the "extra" state of an allocation and make the pointers | ||
/// it contains (in relocations) tagged. The way we construct allocations is | ||
/// to always first construct it without extra and then add the extra. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -132,7 +132,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | |
/// | ||
/// There is no separate `eval_rvalue` function. Instead, the code for handling each rvalue | ||
/// type writes its results directly into the memory specified by the place. | ||
fn eval_rvalue_into_place( | ||
pub fn eval_rvalue_into_place( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, why this? We're trying to keep a clean interface for the interpreter, that'll never work if everyone just adds There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the const_prop changes, it seems like what happened is that const_prop moved up a layer or two of the interpreter abstractions. Shouldn't that mean that while some new operations need to be public now, some of the previously used one can be made private? If you could scan the functions that const_prop no longer needs and make them private, that would make me much less concerned about this change. :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, I think you're correct. I'll do that in the other PR as well. |
||
&mut self, | ||
rvalue: &mir::Rvalue<'tcx>, | ||
place: &mir::Place<'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.
Why is "unsupported" the right category for this?
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.
To me, it seemed similar to some of the other "unsupported" cases like
ReadForeignStatic
,NoMirFor
,DerefFunctionPointer
,OutOfTls
etc in that it represents allowed behavior in a valid program that simply isn't implemented (or can't be implemented) in miri. I dismissedInvalidProgramInfo
because this doesn't indicate an invalid program, however reading the doc comment, it sounds like that may actually be a more appropriate classification. Thoughts?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.
Looks like @oli-obk is already working on removing this again so whatever.