Merge #161

161: Don't expose bit flag operations quite so much to the user. r=vext01 a=ltratt

Using 'bitflags' gives us the same internal representation but slightly more static type safety.

Co-authored-by: Laurence Tratt <laurie@tratt.net>

ykpack/Cargo.toml

@@ -7,5 +7,6 @@ license = "Apache-2.0 OR MIT"
 
 [dependencies]
 bincode = "1.3.1"
+bitflags = "1.2"
 fallible-iterator = "0.2.0"
 serde = { version = "1.0.115", features = ["derive"] }

ykpack/src/lib.rs

@@ -37,7 +37,7 @@ pub const SIR_SECTION_PREFIX: &str = ".yksir_";
 
 #[cfg(test)]
 mod tests {
-    use super::{BasicBlock, Body, Decoder, Encoder, Pack, Statement, Terminator};
+    use super::{BasicBlock, Body, BodyFlags, Decoder, Encoder, Pack, Statement, Terminator};
     use fallible_iterator::{self, FallibleIterator};
     use std::io::{Cursor, Seek, SeekFrom};
 
@@ -66,7 +66,7 @@ mod tests {
         let sir1 = Pack::Body(Body {
             symbol_name: String::from("symbol1"),
             blocks: blocks1,
-            flags: 0,
+            flags: BodyFlags::empty(),
             local_decls: Vec::new(),
             num_args: 0,
         });
@@ -82,7 +82,7 @@ mod tests {
         let sir2 = Pack::Body(Body {
             symbol_name: String::from("symbol2"),
             blocks: blocks2,
-            flags: 0,
+            flags: BodyFlags::empty(),
             local_decls: Vec::new(),
             num_args: 0,
         });

ykpack/src/types.rs

@@ -1,5 +1,6 @@
 //! Types for the Yorick intermediate language.
 
+use bitflags::bitflags;
 use serde::{Deserialize, Serialize};
 use std::{
     convert::TryFrom,
@@ -343,14 +344,17 @@ impl Display for Local {
     }
 }
 
-/// Bits in the `flags` bitfield in `Body`.
-pub mod bodyflags {
-    /// This function is annotated #[do_not_trace].
-    pub const DO_NOT_TRACE: u8 = 1;
-    /// This function is annotated #[interp_step].
-    pub const INTERP_STEP: u8 = 1 << 1;
-    /// This function is yktrace::trace_debug.
-    pub const TRACE_DEBUG: u8 = 1 << 2;
+bitflags! {
+    /// Bits in the `flags` bitfield in `Body`.
+    #[derive(Serialize, Deserialize)]
+    pub struct BodyFlags: u8 {
+        /// This function is annotated #[do_not_trace].
+        const DO_NOT_TRACE = 0b00000001;
+        /// This function is annotated #[interp_step].
+        const INTERP_STEP = 0b00000010;
+        /// This function is yktrace::trace_debug.
+        const TRACE_DEBUG = 0b00000100;
+    }
 }
 
 /// The definition of a local variable, including its type.
@@ -374,15 +378,15 @@ impl Display for LocalDecl {
 pub struct Body {
     pub symbol_name: String,
     pub blocks: Vec<BasicBlock>,
-    pub flags: u8,
+    pub flags: BodyFlags,
     pub local_decls: Vec<LocalDecl>,
     pub num_args: usize,
 }
 
 impl Display for Body {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         writeln!(f, "symbol: {}", self.symbol_name)?;
-        writeln!(f, "  flags: {}", self.flags)?;
+        writeln!(f, "  flags: {:?}", self.flags)?;
         writeln!(f, "  num_args: {}", self.num_args)?;
 
         writeln!(f, "  local_decls:")?;

yktrace/src/sir.rs

@@ -13,7 +13,7 @@ use std::{
     iter::Iterator,
     path::Path
 };
-use ykpack::{self, bodyflags, Body, CguHash, Decoder, Pack, Ty};
+use ykpack::{self, Body, BodyFlags, CguHash, Decoder, Pack, Ty};
 
 /// The serialised IR loaded in from disk. One of these structures is generated in the above
 /// `lazy_static` and is shared immutably for all threads.
@@ -144,7 +144,7 @@ pub fn sir_trace_str(sir: &Sir, trace: &dyn SirTrace, trimmed: bool, show_blocks
 
         let body = sir.bodies.get(&loc.symbol_name);
         if let Some(body) = body {
-            if body.flags & bodyflags::INTERP_STEP != 0 {
+            if body.flags.contains(BodyFlags::INTERP_STEP) {
                 write!(res_r, "INTERP_STEP ").unwrap();
             }
         }

yktrace/src/tir.rs

@@ -14,8 +14,8 @@ use std::{
     fmt::{self, Display, Write}
 };
 pub use ykpack::{
-    BinOp, CallOperand, Constant, ConstantInt, IPlace, Local, LocalDecl, LocalIndex, Ptr,
-    SignedInt, Statement, Terminator, UnsignedInt
+    BinOp, BodyFlags, CallOperand, Constant, ConstantInt, IPlace, Local, LocalDecl, LocalIndex,
+    Ptr, SignedInt, Statement, Terminator, UnsignedInt
 };
 
 const RETURN_LOCAL: Local = Local(0);
@@ -101,7 +101,7 @@ impl<'a> TirTrace<'a> {
             // Ignore yktrace::trace_debug.
             // We don't use the 'ignore' machinery below, as that would require the TraceDebugCall
             // terminator to contain the symbol name, which would be wasteful.
-            if body.flags & ykpack::bodyflags::TRACE_DEBUG != 0 {
+            if body.flags.contains(BodyFlags::TRACE_DEBUG) {
                 continue;
             }
 
@@ -211,7 +211,7 @@ impl<'a> TirTrace<'a> {
             {
                 if let Some(callee_sym) = op.symbol() {
                     if let Some(callee_body) = sir.bodies.get(callee_sym) {
-                        if callee_body.flags & ykpack::bodyflags::INTERP_STEP != 0 {
+                        if callee_body.flags.contains(BodyFlags::INTERP_STEP) {
                             if in_interp_step {
                                 panic!("recursion into interp_step detected");
                             }
@@ -255,7 +255,7 @@ impl<'a> TirTrace<'a> {
 
                             // If the function has been annotated with do_not_trace, turn it into a
                             // call.
-                            if callbody.flags & ykpack::bodyflags::DO_NOT_TRACE != 0 {
+                            if callbody.flags.contains(BodyFlags::DO_NOT_TRACE) {
                                 ignore = Some(callee_sym.to_string());
                                 term_stmts.push(Statement::Call(op.clone(), newargs, Some(ret_val)))
                             } else {
@@ -288,7 +288,7 @@ impl<'a> TirTrace<'a> {
                     }
                 }
                 Terminator::Return => {
-                    if body.flags & ykpack::bodyflags::INTERP_STEP != 0 {
+                    if body.flags.contains(BodyFlags::INTERP_STEP) {
                         debug_assert!(in_interp_step);
                         in_interp_step = false;
                         continue;