chore(ssa): Add doc comments on Opcode struct (#778)

* add small comments

* Be explicit about the cases which should panic the compiler

* formatting

* make AND, XOR and RANGE unreachable

crates/noirc_evaluator/src/ssa/builtin.rs

@@ -3,6 +3,10 @@ use acvm::{acir::BlackBoxFunc, FieldElement};
 use num_bigint::BigUint;
 use num_traits::{One, Zero};
 
+/// Opcode here refers to either a black box function
+/// defined in ACIR, or a function which has its
+/// function signature defined in Noir, but its
+/// function definition is implemented in the compiler.
 #[derive(Clone, Debug, Hash, Copy, PartialEq, Eq)]
 pub enum Opcode {
     LowLevel(BlackBoxFunc),
@@ -17,6 +21,11 @@ impl std::fmt::Display for Opcode {
 }
 
 impl Opcode {
+    /// Searches for an `Opcode` using its
+    /// string equivalent name.
+    ///
+    /// Returns `None` if there is no string that
+    /// corresponds to any of the opcodes.
     pub fn lookup(op_name: &str) -> Option<Opcode> {
         match op_name {
             "to_le_bits" => Some(Opcode::ToBits),
@@ -25,7 +34,7 @@ impl Opcode {
         }
     }
 
-    pub fn name(&self) -> &str {
+    fn name(&self) -> &str {
         match self {
             Opcode::LowLevel(op) => op.name(),
             Opcode::ToBits => "to_le_bits",
@@ -37,38 +46,46 @@ impl Opcode {
         match self {
             Opcode::LowLevel(op) => {
                 match op {
+                    // Pointers do not overflow
                     BlackBoxFunc::SHA256
                     | BlackBoxFunc::Blake2s
                     | BlackBoxFunc::Pedersen
-                    | BlackBoxFunc::FixedBaseScalarMul => BigUint::zero(), //pointers do not overflow
+                    | BlackBoxFunc::FixedBaseScalarMul => BigUint::zero(),
+                    // Verify returns zero or one
                     BlackBoxFunc::SchnorrVerify
                     | BlackBoxFunc::EcdsaSecp256k1
-                    | BlackBoxFunc::MerkleMembership => BigUint::one(), //verify returns 0 or 1
+                    | BlackBoxFunc::MerkleMembership => BigUint::one(),
                     BlackBoxFunc::HashToField128Security => ObjectType::NativeField.max_size(),
-                    _ => todo!("max value must be implemented for opcode {} ", op),
+                    BlackBoxFunc::AES => {
+                        todo!("ICE: AES is unimplemented")
+                    }
+                    BlackBoxFunc::RANGE | BlackBoxFunc::AND | BlackBoxFunc::XOR => {
+                        unimplemented!("ICE: these opcodes do not have Noir builtin functions")
+                    }
                 }
             }
             Opcode::ToBits | Opcode::ToRadix => BigUint::zero(), //pointers do not overflow
         }
     }
 
-    //Returns the number of elements and their type, of the output result corresponding to the OPCODE function.
+    /// Returns the number of elements that the `Opcode` should return
+    /// and the type.
     pub fn get_result_type(&self) -> (u32, ObjectType) {
         match self {
             Opcode::LowLevel(op) => {
                 match op {
-                    BlackBoxFunc::AES => (0, ObjectType::NotAnObject), //Not implemented
-                    BlackBoxFunc::SHA256 => (32, ObjectType::Unsigned(8)),
-                    BlackBoxFunc::Blake2s => (32, ObjectType::Unsigned(8)),
+                    BlackBoxFunc::AES => todo!("ICE: AES is unimplemented"),
+                    BlackBoxFunc::SHA256 | BlackBoxFunc::Blake2s => (32, ObjectType::Unsigned(8)),
                     BlackBoxFunc::HashToField128Security => (1, ObjectType::NativeField),
-                    BlackBoxFunc::MerkleMembership => (1, ObjectType::NativeField), //or bool?
-                    BlackBoxFunc::SchnorrVerify => (1, ObjectType::NativeField),    //or bool?
+                    // See issue #775 on changing this to return a boolean
+                    BlackBoxFunc::MerkleMembership
+                    | BlackBoxFunc::SchnorrVerify
+                    | BlackBoxFunc::EcdsaSecp256k1 => (1, ObjectType::NativeField),
                     BlackBoxFunc::Pedersen => (2, ObjectType::NativeField),
-                    BlackBoxFunc::EcdsaSecp256k1 => (1, ObjectType::NativeField), //field?
                     BlackBoxFunc::FixedBaseScalarMul => (2, ObjectType::NativeField),
-                    BlackBoxFunc::AND => (1, ObjectType::NativeField),
-                    BlackBoxFunc::XOR => (1, ObjectType::NativeField),
-                    BlackBoxFunc::RANGE => (0, ObjectType::NotAnObject),
+                    BlackBoxFunc::RANGE | BlackBoxFunc::AND | BlackBoxFunc::XOR => {
+                        unreachable!("ICE: these opcodes do not have Noir builtin functions")
+                    }
                 }
             }
             Opcode::ToBits => (FieldElement::max_num_bits(), ObjectType::Boolean),