two small changes in evm (#5700)

* add a simple test for is_push so that every small utility has a corresponding unit test

* split evm/factory.rs into two files as the TODO suggests

* style fixes according to review comments

ethcore/src/evm/factory.rs

@@ -16,70 +16,11 @@
 
 //! Evm factory.
 //!
-//! TODO: consider spliting it into two separate files.
-use std::fmt;
 use std::sync::Arc;
 use evm::Evm;
 use util::U256;
 use super::interpreter::SharedCache;
-
+use super::vmtype::VMType;
-#[derive(Debug, PartialEq, Clone)]
-/// Type of EVM to use.
-pub enum VMType {
-	/// JIT EVM
-	#[cfg(feature = "jit")]
-	Jit,
-	/// RUST EVM
-	Interpreter
-}
-
-impl fmt::Display for VMType {
-	#[cfg(feature="jit")]
-	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		write!(f, "{}", match *self {
-			VMType::Jit => "JIT",
-			VMType::Interpreter => "INT"
-		})
-	}
-	#[cfg(not(feature="jit"))]
-	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-		write!(f, "{}", match *self {
-			VMType::Interpreter => "INT"
-		})
-	}
-}
-
-impl Default for VMType {
-	fn default() -> Self {
-		VMType::Interpreter
-	}
-}
-
-impl VMType {
-	/// Return all possible VMs (JIT, Interpreter)
-	#[cfg(feature = "jit")]
-	pub fn all() -> Vec<VMType> {
-		vec![VMType::Jit, VMType::Interpreter]
-	}
-
-	/// Return all possible VMs (Interpreter)
-	#[cfg(not(feature = "jit"))]
-	pub fn all() -> Vec<VMType> {
-		vec![VMType::Interpreter]
-	}
-
-	/// Return new jit if it's possible
-	#[cfg(not(feature = "jit"))]
-	pub fn jit() -> Option<Self> {
-		None
-	}
-
-	/// Return new jit if it's possible
-	#[cfg(feature = "jit")]
-	pub fn jit() -> Option<Self> {
-		Some(VMType::Jit)
-	}
-}
 
 /// Evm factory. Creates appropriate Evm.
 #[derive(Clone)]

ethcore/src/evm/instructions.rs

@@ -23,6 +23,13 @@ pub fn is_push(i: Instruction) -> bool {
 	i >= PUSH1 && i <= PUSH32
 }
 
+#[test]
+fn test_is_push() {
+	assert!(is_push(PUSH1));
+	assert!(is_push(PUSH32));
+	assert!(!is_push(DUP1));
+}
+
 /// Returns number of bytes to read for `PUSHN` instruction
 /// PUSH1 -> 1
 pub fn get_push_bytes(i: Instruction) -> usize {

ethcore/src/evm/mod.rs

@@ -22,6 +22,8 @@ pub mod interpreter;
 #[macro_use]
 pub mod factory;
 pub mod schedule;
+
+mod vmtype;
 mod instructions;
 #[cfg(feature = "jit" )]
 mod jit;
@@ -33,6 +35,7 @@ mod benches;
 
 pub use self::evm::{Evm, Error, Finalize, FinalizationResult, GasLeft, Result, CostType};
 pub use self::ext::{Ext, ContractCreateResult, MessageCallResult, CreateContractAddress};
-pub use self::factory::{Factory, VMType};
+pub use self::vmtype::VMType;
+pub use self::factory::Factory;
 pub use self::schedule::Schedule;
 pub use types::executed::CallType;

ethcore/src/evm/vmtype.rs

@@ -0,0 +1,75 @@
+// Copyright 2015-2017 Parity Technologies (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+use std::fmt;
+
+/// Type of EVM to use.
+#[derive(Debug, PartialEq, Clone)]
+pub enum VMType {
+	/// JIT EVM
+	#[cfg(feature = "jit")]
+	Jit,
+	/// RUST EVM
+	Interpreter
+}
+
+impl fmt::Display for VMType {
+	#[cfg(feature="jit")]
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		write!(f, "{}", match *self {
+			VMType::Jit => "JIT",
+			VMType::Interpreter => "INT"
+		})
+	}
+	#[cfg(not(feature="jit"))]
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		write!(f, "{}", match *self {
+			VMType::Interpreter => "INT"
+		})
+	}
+}
+
+impl Default for VMType {
+	fn default() -> Self {
+		VMType::Interpreter
+	}
+}
+
+impl VMType {
+	/// Return all possible VMs (JIT, Interpreter)
+	#[cfg(feature = "jit")]
+	pub fn all() -> Vec<VMType> {
+		vec![VMType::Jit, VMType::Interpreter]
+	}
+
+	/// Return all possible VMs (Interpreter)
+	#[cfg(not(feature = "jit"))]
+	pub fn all() -> Vec<VMType> {
+		vec![VMType::Interpreter]
+	}
+
+	/// Return new jit if it's possible
+	#[cfg(not(feature = "jit"))]
+	pub fn jit() -> Option<Self> {
+		None
+	}
+
+	/// Return new jit if it's possible
+	#[cfg(feature = "jit")]
+	pub fn jit() -> Option<Self> {
+		Some(VMType::Jit)
+	}
+}