Skip to content

Rustfmt some code (WIP) #6

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
+372 −250
Closed

Rustfmt some code (WIP) #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
effaff4
Rustfmt: Good Changes
killercup Apr 16, 2016
b653fd2
Rustfmt: Ambiguous Changes
killercup Apr 16, 2016
fa2c660
Rustfmt: Bad Changes
killercup Apr 16, 2016
5a03b95
Rustfmt: Fix Actual Errors
killercup Apr 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,26 @@ pub type EvalResult<T> = Result<T, EvalError>;
impl Error for EvalError {
fn description(&self) -> &str {
match *self {
EvalError::DanglingPointerDeref =>
"dangling pointer was dereferenced",
EvalError::InvalidBool =>
"invalid boolean value read",
EvalError::InvalidDiscriminant =>
"invalid enum discriminant value read",
EvalError::PointerOutOfBounds =>
"pointer offset outside bounds of allocation",
EvalError::ReadPointerAsBytes =>
"a raw memory access tried to access part of a pointer value as raw bytes",
EvalError::ReadBytesAsPointer =>
"attempted to interpret some raw bytes as a pointer address",
EvalError::InvalidPointerMath =>
"attempted to do math or a comparison on pointers into different allocations",
EvalError::ReadUndefBytes =>
"attempted to read undefined bytes",
EvalError::DanglingPointerDeref => "dangling pointer was dereferenced",
EvalError::InvalidBool => "invalid boolean value read",
EvalError::InvalidDiscriminant => "invalid enum discriminant value read",
EvalError::PointerOutOfBounds => "pointer offset outside bounds of allocation",
EvalError::ReadPointerAsBytes => {
"a raw memory access tried to access part of a pointer value as raw bytes"
}
EvalError::ReadBytesAsPointer => {
"attempted to interpret some raw bytes as a pointer address"
}
EvalError::InvalidPointerMath => {
"attempted to do math or a comparison on pointers into different allocations"
}
EvalError::ReadUndefBytes => "attempted to read undefined bytes",
Copy link
Member

Choose a reason for hiding this comment

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

Yeah... this makes it less consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I don't like it either because it messes with consistency.

(Setting format_strings = false seems to solve this, by the way, but I didn't want to add a rustfmt.toml in this PR.)

}
}

fn cause(&self) -> Option<&Error> { None }
fn cause(&self) -> Option<&Error> {
None
}
}

impl fmt::Display for EvalError {
Expand Down
369 changes: 212 additions & 157 deletions src/interpreter.rs
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

// From rustc.
extern crate arena;
#[macro_use] extern crate rustc;
#[macro_use]
extern crate rustc;
Copy link

Choose a reason for hiding this comment

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

IMO, this is actually a good change, for consistency. Attributes usually go above statements. You'd never write:

#[inline] fn test() {
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point.

I would put empty lines around the #[macro_use] extern crate line though. And re-order the imports so that the #[macro_use] ones come first.

Copy link

Choose a reason for hiding this comment

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

You mean like this?

#[macro_use]
extern crate rustc;

extern crate arena;
extern crate rustc_data_structures;
extern crate rustc_mir;
extern crate syntax;

I agree that putting a newline before attributes makes sense.

However, re-ordering statements isn't rustfmt can reliably do due to comments. That is, there's no way to determine if // From rustc. should stick to extern crate arena; or if it should stay where it is.

Copy link
Member

Choose a reason for hiding this comment

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

I'm mostly ambivalent about this change. I used the same-line #[macro_use] after seeing it in other projects including rustc. However, checking now, it's not even consistent within rustc.

On the other hand, I want to keep the list of crates alphabetical.

extern crate rustc_data_structures;
extern crate rustc_mir;
extern crate syntax;
Expand Down
137 changes: 83 additions & 54 deletions src/memory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ use std::{fmt, iter, mem, ptr};
use error::{EvalError, EvalResult};
use primval::PrimVal;

////////////////////////////////////////////////////////////////////////////////
// Value representations
////////////////////////////////////////////////////////////////////////////////
/// /////////////////////////////////////////////////////////////////////////////
/// Value representations
/// /////////////////////////////////////////////////////////////////////////////
Copy link

Choose a reason for hiding this comment

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

I'd consider these errors. It's a silent modification of the documentation.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, these aren't doc comments. Definitely an error.

Copy link
Member Author

Choose a reason for hiding this comment

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

cf. rust-lang/rustfmt#652


#[derive(Clone, Debug, Eq, PartialEq)]
pub enum Repr {
/// Representation for a non-aggregate type such as a boolean, integer, character or pointer.
Primitive {
size: usize
size: usize,
},

/// The representation for aggregate types including structs, enums, and tuples.
Expand Down Expand Up @@ -54,9 +54,9 @@ impl Repr {
}
}

////////////////////////////////////////////////////////////////////////////////
// Allocations and pointers
////////////////////////////////////////////////////////////////////////////////
/// /////////////////////////////////////////////////////////////////////////////
/// Allocations and pointers
/// /////////////////////////////////////////////////////////////////////////////

#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)]
pub struct AllocId(u64);
Expand Down Expand Up @@ -86,9 +86,9 @@ impl Pointer {
}
}

////////////////////////////////////////////////////////////////////////////////
// Top-level interpreter memory
////////////////////////////////////////////////////////////////////////////////
/// /////////////////////////////////////////////////////////////////////////////
/// Top-level interpreter memory
/// /////////////////////////////////////////////////////////////////////////////

pub struct Memory {
alloc_map: HashMap<AllocId, Allocation>,
Expand Down Expand Up @@ -163,9 +163,9 @@ impl Memory {
Ok(())
}

////////////////////////////////////////////////////////////////////////////////
// Allocation accessors
////////////////////////////////////////////////////////////////////////////////
/// /////////////////////////////////////////////////////////////////////////////
/// Allocation accessors
/// /////////////////////////////////////////////////////////////////////////////

pub fn get(&self, id: AllocId) -> EvalResult<&Allocation> {
self.alloc_map.get(&id).ok_or(EvalError::DanglingPointerDeref)
Expand Down Expand Up @@ -216,17 +216,19 @@ impl Memory {
let relocation_width = (self.pointer_size - 1) * 3;
for (i, target_id) in relocations {
print!("{:1$}", "", (i - pos) * 3);
print!("└{0:─^1$}┘ ", format!("({})", target_id), relocation_width);
print!("└{0:─^1$}┘ ",
format!("({})", target_id),
relocation_width);
pos = i + self.pointer_size;
}
println!("");
}
}
}

////////////////////////////////////////////////////////////////////////////////
// Byte accessors
////////////////////////////////////////////////////////////////////////////////
/// /////////////////////////////////////////////////////////////////////////////
/// Byte accessors
/// /////////////////////////////////////////////////////////////////////////////

fn get_bytes_unchecked(&self, ptr: Pointer, size: usize) -> EvalResult<&[u8]> {
let alloc = try!(self.get(ptr.alloc_id));
Expand Down Expand Up @@ -258,9 +260,9 @@ impl Memory {
self.get_bytes_unchecked_mut(ptr, size)
}

////////////////////////////////////////////////////////////////////////////////
// Reading and writing
////////////////////////////////////////////////////////////////////////////////
/// /////////////////////////////////////////////////////////////////////////////
/// Reading and writing
/// /////////////////////////////////////////////////////////////////////////////

pub fn copy(&mut self, src: Pointer, dest: Pointer, size: usize) -> EvalResult<()> {
try!(self.check_relocation_edges(src, size));
Expand Down Expand Up @@ -297,7 +299,9 @@ impl Memory {

pub fn write_repeat(&mut self, ptr: Pointer, val: u8, count: usize) -> EvalResult<()> {
let bytes = try!(self.get_bytes_mut(ptr, count));
for b in bytes { *b = val; }
for b in bytes {
*b = val;
}
Ok(())
}

Expand All @@ -309,10 +313,16 @@ impl Memory {
let size = self.pointer_size;
try!(self.check_defined(ptr, size));
let offset = try!(self.get_bytes_unchecked(ptr, size))
.read_uint::<NativeEndian>(size).unwrap() as usize;
.read_uint::<NativeEndian>(size)
.unwrap() as usize;
let alloc = try!(self.get(ptr.alloc_id));
match alloc.relocations.get(&ptr.offset) {
Some(&alloc_id) => Ok(Pointer { alloc_id: alloc_id, offset: offset }),
Some(&alloc_id) => {
Ok(Pointer {
alloc_id: alloc_id,
offset: offset,
})
}
None => Err(EvalError::ReadBytesAsPointer),
}
}
Expand All @@ -331,14 +341,14 @@ impl Memory {
let pointer_size = self.pointer_size;
match val {
PrimVal::Bool(b) => self.write_bool(ptr, b),
PrimVal::I8(n) => self.write_int(ptr, n as i64, 1),
PrimVal::I16(n) => self.write_int(ptr, n as i64, 2),
PrimVal::I32(n) => self.write_int(ptr, n as i64, 4),
PrimVal::I64(n) => self.write_int(ptr, n as i64, 8),
PrimVal::U8(n) => self.write_uint(ptr, n as u64, 1),
PrimVal::U16(n) => self.write_uint(ptr, n as u64, 2),
PrimVal::U32(n) => self.write_uint(ptr, n as u64, 4),
PrimVal::U64(n) => self.write_uint(ptr, n as u64, 8),
PrimVal::I8(n) => self.write_int(ptr, n as i64, 1),
PrimVal::I16(n) => self.write_int(ptr, n as i64, 2),
PrimVal::I32(n) => self.write_int(ptr, n as i64, 4),
PrimVal::I64(n) => self.write_int(ptr, n as i64, 8),
PrimVal::U8(n) => self.write_uint(ptr, n as u64, 1),
PrimVal::U16(n) => self.write_uint(ptr, n as u64, 2),
PrimVal::U32(n) => self.write_uint(ptr, n as u64, 4),
PrimVal::U64(n) => self.write_uint(ptr, n as u64, 8),
PrimVal::IntegerPtr(n) => self.write_uint(ptr, n as u64, pointer_size),
PrimVal::AbstractPtr(_p) => unimplemented!(),
}
Expand Down Expand Up @@ -391,13 +401,14 @@ impl Memory {
self.write_uint(ptr, n, size)
}

////////////////////////////////////////////////////////////////////////////////
// Relocations
////////////////////////////////////////////////////////////////////////////////
/// /////////////////////////////////////////////////////////////////////////////
/// Relocations
/// /////////////////////////////////////////////////////////////////////////////

fn relocations(&self, ptr: Pointer, size: usize)
-> EvalResult<btree_map::Range<usize, AllocId>>
{
fn relocations(&self,
ptr: Pointer,
size: usize)
-> EvalResult<btree_map::Range<usize, AllocId>> {
let start = ptr.offset.saturating_sub(self.pointer_size - 1);
let end = start + size;
Ok(try!(self.get(ptr.alloc_id)).relocations.range(Included(&start), Excluded(&end)))
Expand All @@ -406,7 +417,9 @@ impl Memory {
fn clear_relocations(&mut self, ptr: Pointer, size: usize) -> EvalResult<()> {
// Find all relocations overlapping the given range.
let keys: Vec<_> = try!(self.relocations(ptr, size)).map(|(&k, _)| k).collect();
if keys.len() == 0 { return Ok(()); }
if keys.len() == 0 {
return Ok(());
}

// Find the start and end of the given range and its outermost relocations.
let start = ptr.offset;
Expand All @@ -418,11 +431,17 @@ impl Memory {

// Mark parts of the outermost relocations as undefined if they partially fall outside the
// given range.
if first < start { alloc.undef_mask.set_range(first, start, false); }
if last > end { alloc.undef_mask.set_range(end, last, false); }
if first < start {
alloc.undef_mask.set_range(first, start, false);
}
if last > end {
alloc.undef_mask.set_range(end, last, false);
}

// Forget all the relocations.
for k in keys { alloc.relocations.remove(&k); }
for k in keys {
alloc.relocations.remove(&k);
}

Ok(())
}
Expand All @@ -447,9 +466,9 @@ impl Memory {
Ok(())
}

////////////////////////////////////////////////////////////////////////////////
// Undefined bytes
////////////////////////////////////////////////////////////////////////////////
/// /////////////////////////////////////////////////////////////////////////////
/// Undefined bytes
/// /////////////////////////////////////////////////////////////////////////////

// FIXME(tsino): This is a very naive, slow version.
fn copy_undef_mask(&mut self, src: Pointer, dest: Pointer, size: usize) -> EvalResult<()> {
Expand All @@ -473,18 +492,20 @@ impl Memory {
Ok(())
}

pub fn mark_definedness(&mut self, ptr: Pointer, size: usize, new_state: bool)
-> EvalResult<()>
{
pub fn mark_definedness(&mut self,
ptr: Pointer,
size: usize,
new_state: bool)
-> EvalResult<()> {
let mut alloc = try!(self.get_mut(ptr.alloc_id));
alloc.undef_mask.set_range(ptr.offset, ptr.offset + size, new_state);
Ok(())
}
}

////////////////////////////////////////////////////////////////////////////////
// Undefined byte tracking
////////////////////////////////////////////////////////////////////////////////
/// /////////////////////////////////////////////////////////////////////////////
/// Undefined byte tracking
/// /////////////////////////////////////////////////////////////////////////////

type Block = u64;
const BLOCK_SIZE: usize = 64;
Expand All @@ -507,21 +528,29 @@ impl UndefMask {

/// Check whether the range `start..end` (end-exclusive) is entirely defined.
fn is_range_defined(&self, start: usize, end: usize) -> bool {
if end > self.len { return false; }
if end > self.len {
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

This and the follow few diffs seem to make the code bigger for no great readability improvement.

for i in start..end {
if !self.get(i) { return false; }
if !self.get(i) {
return false;
}
}
true
}

fn set_range(&mut self, start: usize, end: usize, new_state: bool) {
let len = self.len;
if end > len { self.grow(end - len, new_state); }
if end > len {
self.grow(end - len, new_state);
}
self.set_range_inbounds(start, end, new_state);
}

fn set_range_inbounds(&mut self, start: usize, end: usize, new_state: bool) {
for i in start..end { self.set(i, new_state); }
for i in start..end {
self.set(i, new_state);
}
}

fn get(&self, i: usize) -> bool {
Expand Down
25 changes: 16 additions & 9 deletions src/primval.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@ use memory::Pointer;
#[derive(Clone, Copy, Debug, PartialEq)]
pub enum PrimVal {
Bool(bool),
I8(i8), I16(i16), I32(i32), I64(i64),
U8(u8), U16(u16), U32(u32), U64(u64),
I8(i8),
I16(i16),
I32(i32),
I64(i64),
U8(u8),
U16(u16),
U32(u32),
U64(u64),

AbstractPtr(Pointer),
IntegerPtr(u64),
Expand Down Expand Up @@ -54,19 +60,20 @@ pub fn binary_op(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> EvalResul

use self::PrimVal::*;
let val = match (left, right) {
(I8(l), I8(r)) => int_binops!(I8, l, r),
(I8(l), I8(r)) => int_binops!(I8, l, r),
(I16(l), I16(r)) => int_binops!(I16, l, r),
(I32(l), I32(r)) => int_binops!(I32, l, r),
(I64(l), I64(r)) => int_binops!(I64, l, r),
(U8(l), U8(r)) => int_binops!(U8, l, r),
(U8(l), U8(r)) => int_binops!(U8, l, r),
(U16(l), U16(r)) => int_binops!(U16, l, r),
(U32(l), U32(r)) => int_binops!(U32, l, r),
(U64(l), U64(r)) => int_binops!(U64, l, r),

(IntegerPtr(l), IntegerPtr(r)) => int_binops!(IntegerPtr, l, r),

(AbstractPtr(_), IntegerPtr(_)) | (IntegerPtr(_), AbstractPtr(_)) =>
return unrelated_ptr_ops(bin_op),
(AbstractPtr(_), IntegerPtr(_)) | (IntegerPtr(_), AbstractPtr(_)) => {
return unrelated_ptr_ops(bin_op)
}

(AbstractPtr(l_ptr), AbstractPtr(r_ptr)) => {
if l_ptr.alloc_id != r_ptr.alloc_id {
Expand Down Expand Up @@ -99,15 +106,15 @@ pub fn unary_op(un_op: mir::UnOp, val: PrimVal) -> PrimVal {
use self::PrimVal::*;
match (un_op, val) {
(Not, Bool(b)) => Bool(!b),
(Not, I8(n)) => I8(!n),
(Neg, I8(n)) => I8(-n),
(Not, I8(n)) => I8(!n),
(Neg, I8(n)) => I8(-n),
(Not, I16(n)) => I16(!n),
(Neg, I16(n)) => I16(-n),
(Not, I32(n)) => I32(!n),
(Neg, I32(n)) => I32(-n),
(Not, I64(n)) => I64(!n),
(Neg, I64(n)) => I64(-n),
(Not, U8(n)) => U8(!n),
(Not, U8(n)) => U8(!n),
(Not, U16(n)) => U16(!n),
(Not, U32(n)) => U32(!n),
(Not, U64(n)) => U64(!n),
Expand Down
Loading