chore: resolve builtin/foreign attribute names to a shared enum

compiler/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -9,6 +9,7 @@
 };
 use iter_extended::vecmap;
 use noirc_frontend::hir_def::types::Type as HirType;
+use noirc_frontend::shared::Builtin;
 
 use crate::ssa::{ir::integer::IntegerConstant, opt::pure::Purity};
 
@@ -323,39 +324,52 @@
     /// Lookup an Intrinsic by name and return it if found.
     /// If there is no such intrinsic by that name, None is returned.
     ///
-    /// `noirc_frontend::ownership::builtin_supports_clone_elision` keeps a name-based
-    /// copy of the clone-elision classification of these intrinsics (it cannot depend
-    /// on this crate). When adding or reclassifying a name here, update it as well;
+    /// This is only used where a name arrives as text (e.g. the SSA parser); the
+    /// compilation pipeline resolves names to [`Builtin`] once, in the frontend,
+    /// and maps them here via [`Self::from_builtin`].
+    pub(crate) fn lookup(name: &str) -> Option<Intrinsic> {
+        Builtin::lookup(name).and_then(Self::from_builtin)
+    }
+
+    /// Map a frontend [`Builtin`] to the [`Intrinsic`] implementing it, or `None`
+    /// for builtins that never reach SSA (comptime-only builtins, and those the
+    /// monomorphizer evaluates away).
+    ///
+    /// `noirc_frontend::ownership::builtin_supports_clone_elision` keeps its own
+    /// clone-elision classification of these builtins (it cannot depend on this
+    /// crate). When adding or reclassifying a builtin here, update it as well;
     /// `ownership_clone_elision_list_matches_intrinsic_purity` in `ssa_gen::tests`
     /// checks the two agree.
-    pub(crate) fn lookup(name: &str) -> Option<Intrinsic> {
-        match name {
-            "array_len" => Some(Intrinsic::ArrayLen),
-            "array_as_str_unchecked" => Some(Intrinsic::ArrayAsStrUnchecked),
-            "as_vector" => Some(Intrinsic::AsVector),
-            "assert_constant" => Some(Intrinsic::AssertConstant),
-            "static_assert" => Some(Intrinsic::StaticAssert),
-            "apply_range_constraint" => Some(Intrinsic::ApplyRangeConstraint),
-            "vector_push_back" => Some(Intrinsic::VectorPushBack),
-            "vector_push_front" => Some(Intrinsic::VectorPushFront),
-            "vector_pop_back" => Some(Intrinsic::VectorPopBack),
-            "vector_pop_front" => Some(Intrinsic::VectorPopFront),
-            "vector_insert" => Some(Intrinsic::VectorInsert),
-            "vector_remove" => Some(Intrinsic::VectorRemove),
-            "str_as_bytes" => Some(Intrinsic::StrAsBytes),
-            "to_le_radix" => Some(Intrinsic::ToRadix(Endian::Little)),
-            "to_be_radix" => Some(Intrinsic::ToRadix(Endian::Big)),
-            "to_le_bits" => Some(Intrinsic::ToBits(Endian::Little)),
-            "to_be_bits" => Some(Intrinsic::ToBits(Endian::Big)),
-            "as_witness" => Some(Intrinsic::AsWitness),
-            "is_unconstrained" => Some(Intrinsic::IsUnconstrained),
-            "derive_pedersen_generators" => Some(Intrinsic::DerivePedersenGenerators),
-            "field_less_than" => Some(Intrinsic::FieldLessThan),
-            "black_box" => Some(Intrinsic::Hint(Hint::BlackBox)),
-            "array_refcount" => Some(Intrinsic::ArrayRefCount),
-            "vector_refcount" => Some(Intrinsic::VectorRefCount),
-
-            other => BlackBoxFunc::lookup(other).map(Intrinsic::BlackBox),
+    pub(crate) fn from_builtin(builtin: Builtin) -> Option<Intrinsic> {
+        match builtin {
+            Builtin::ArrayLen => Some(Intrinsic::ArrayLen),
+            Builtin::ArrayAsStrUnchecked => Some(Intrinsic::ArrayAsStrUnchecked),
+            Builtin::AsVector => Some(Intrinsic::AsVector),
+            Builtin::AssertConstant => Some(Intrinsic::AssertConstant),
+            Builtin::StaticAssert => Some(Intrinsic::StaticAssert),
+            Builtin::ApplyRangeConstraint => Some(Intrinsic::ApplyRangeConstraint),
+            Builtin::VectorPushBack => Some(Intrinsic::VectorPushBack),
+            Builtin::VectorPushFront => Some(Intrinsic::VectorPushFront),
+            Builtin::VectorPopBack => Some(Intrinsic::VectorPopBack),
+            Builtin::VectorPopFront => Some(Intrinsic::VectorPopFront),
+            Builtin::VectorInsert => Some(Intrinsic::VectorInsert),
+            Builtin::VectorRemove => Some(Intrinsic::VectorRemove),
+            Builtin::StrAsBytes => Some(Intrinsic::StrAsBytes),
+            Builtin::ToLeRadix => Some(Intrinsic::ToRadix(Endian::Little)),
+            Builtin::ToBeRadix => Some(Intrinsic::ToRadix(Endian::Big)),
+            Builtin::ToLeBits => Some(Intrinsic::ToBits(Endian::Little)),
+            Builtin::ToBeBits => Some(Intrinsic::ToBits(Endian::Big)),
+            Builtin::AsWitness => Some(Intrinsic::AsWitness),
+            Builtin::IsUnconstrained => Some(Intrinsic::IsUnconstrained),
+            Builtin::DerivePedersenGenerators => Some(Intrinsic::DerivePedersenGenerators),
+            Builtin::FieldLessThan => Some(Intrinsic::FieldLessThan),
+            Builtin::BlackBoxHint => Some(Intrinsic::Hint(Hint::BlackBox)),
+            Builtin::ArrayRefcount => Some(Intrinsic::ArrayRefCount),
+            Builtin::VectorRefcount => Some(Intrinsic::VectorRefCount),
+
+            Builtin::BlackBox(func) => Some(Intrinsic::BlackBox(func)),
+
+            _ => None,
         }
     }
 }
@@ -454,14 +468,14 @@
     /// An instruction to increment the reference count of a value.
     ///
     /// This currently only has an effect in Brillig code where array sharing and copy on write is
     /// implemented via reference counting. In ACIR code this is done with `imbl::Vector` and these
     /// `IncrementRc` instructions are ignored.
     IncrementRc { value: ValueId },
 
     /// An instruction to decrement the reference count of a value.
     ///
     /// This currently only has an effect in Brillig code where array sharing and copy on write is
     /// implemented via reference counting. In ACIR code this is done with `imbl::Vector` and these
     /// `DecrementRc` instructions are ignored.
     DecrementRc { value: ValueId },
 
@@ -485,7 +499,7 @@
     ///
     /// `typ` should be an array or vector type with an element type
     /// matching each of the `elements` values' types.
     MakeArray { elements: imbl::Vector<ValueId>, typ: Type },
 
     /// A No-op instruction. These are intended to replace other instructions in a block's
     /// instructions vector without having to move each instruction afterward.
@@ -1219,7 +1233,7 @@
 /// Try to avoid mutation until we know something changed, to take advantage of
 /// structural sharing, and avoid needlessly calling `Arc::make_mut` which clones
 /// the content and increases memory use by allocating more pointers on the heap.
 fn im_vec_map_values_mut<T, F>(xs: &mut imbl::Vector<T>, mut f: F)
 where
     T: Copy + PartialEq,
     F: FnMut(T) -> T,
@@ -1285,7 +1299,7 @@
 
         let typ = Type::Array(std::sync::Arc::new(vec![Type::field()]), SemanticLength(2));
         let mut instruction =
             Instruction::MakeArray { elements: imbl::Vector::from(vec![v0, v1]), typ: typ.clone() };
         assert!(instruction.replace_values(&mapping));
         let Instruction::MakeArray { elements, .. } = instruction else { unreachable!() };
         assert_eq!(elements[0], v0);

compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs

@@ -257,10 +257,10 @@
             ast::Definition::Oracle { name, pure } => {
                 self.builder.import_foreign_function(name, *pure).into()
             }
-            ast::Definition::Builtin(name) | ast::Definition::LowLevel(name) => {
-                match self.builder.import_intrinsic(name) {
-                    Some(builtin) => builtin.into(),
-                    None => panic!("No builtin function named '{name}' found"),
+            ast::Definition::Builtin(builtin) | ast::Definition::LowLevel(builtin) => {
+                match Intrinsic::from_builtin(*builtin) {
+                    Some(intrinsic) => self.builder.import_intrinsic_id(intrinsic).into(),
+                    None => panic!("No builtin function named '{builtin}' found"),
                 }
             }
         }
@@ -413,7 +413,7 @@
     ///
     /// The value returned from this function is always that of the allocate instruction.
     fn codegen_array(&mut self, elements: Vec<Values>, typ: Type) -> Values {
         let mut array = imbl::Vector::new();
 
         for element in elements {
             element.for_each(|element| {

compiler/noirc_evaluator/src/ssa/ssa_gen/tests.rs

@@ -458,54 +458,38 @@
 }
 
 /// The ownership pass decides clone elision in `noirc_frontend`, which cannot see
-/// [`Intrinsic`]: it keeps its own name-based copy of the classification. This test
-/// pins the two lists together so a new or reclassified intrinsic cannot silently
-/// diverge from the frontend's view.
+/// [`Intrinsic`](crate::ssa::ir::instruction::Intrinsic): it keeps its own copy of
+/// the classification. This test pins the two together over every [`Builtin`] so a
+/// new or reclassified builtin cannot silently diverge from the frontend's view.
 #[test]
 fn ownership_clone_elision_list_matches_intrinsic_purity() {
+    use crate::ssa::ir::instruction::Intrinsic;
     use crate::ssa::opt::pure::Purity;
     use acvm::acir::BlackBoxFunc;
     use noirc_frontend::ownership::builtin_supports_clone_elision;
+    use noirc_frontend::shared::Builtin;
     use strum::IntoEnumIterator;
 
-    // Keep in sync with the names recognized by `Intrinsic::lookup`.
-    let intrinsic_names = [
-        "array_len",
-        "array_as_str_unchecked",
-        "as_vector",
-        "assert_constant",
-        "static_assert",
-        "apply_range_constraint",
-        "vector_push_back",
-        "vector_push_front",
-        "vector_pop_back",
-        "vector_pop_front",
-        "vector_insert",
-        "vector_remove",
-        "str_as_bytes",
-        "to_le_radix",
-        "to_be_radix",
-        "to_le_bits",
-        "to_be_bits",
-        "as_witness",
-        "is_unconstrained",
-        "derive_pedersen_generators",
-        "field_less_than",
-        "black_box",
-        "array_refcount",
-        "vector_refcount",
-    ];
-    let blackbox_names = BlackBoxFunc::iter().map(|func| func.name().to_string());
-
-    for name in intrinsic_names.into_iter().map(String::from).chain(blackbox_names) {
-        let intrinsic = crate::ssa::ir::instruction::Intrinsic::lookup(&name)
-            .unwrap_or_else(|| panic!("`{name}` should be a known intrinsic"));
-        let elidable = !intrinsic.unsafe_for_clone_elision_in_brillig()
-            && matches!(intrinsic.purity(), Purity::Pure | Purity::PureWithPredicate);
+    // `Builtin::iter` skips the strum-disabled `BlackBox` variant, so chain the
+    // callable black box functions explicitly.
+    let black_boxes = BlackBoxFunc::iter()
+        .filter(|func| Builtin::lookup(func.name()).is_some())
+        .map(Builtin::BlackBox);
+    for builtin in Builtin::iter().chain(black_boxes) {
+        // Builtins that never reach SSA generation have no runtime call whose clone
+        // could be elided; those that do must agree with `Intrinsic`'s purity and
+        // aliasing classification.
+        let elidable = match Intrinsic::from_builtin(builtin) {
+            Some(intrinsic) => {
+                !intrinsic.unsafe_for_clone_elision_in_brillig()
+                    && matches!(intrinsic.purity(), Purity::Pure | Purity::PureWithPredicate)
+            }
+            None => false,
+        };
         assert_eq!(
-            builtin_supports_clone_elision(&name),
+            builtin_supports_clone_elision(builtin),
             elidable,
-            "`builtin_supports_clone_elision` disagrees with `Intrinsic`'s classification of `{name}`",
+            "`builtin_supports_clone_elision` disagrees with `Intrinsic`'s classification of `{builtin}`",
         );
     }
 }
@@ -1173,7 +1157,7 @@
 
 /// `&mut *p` is a re-borrow and must alias the same memory location as `p`.
 #[test]
 fn mut_reborrow_aliases_original_location() {
     let src = "
     fn main() {
         let mut f: u64 = 10;
@@ -1202,7 +1186,7 @@
 
 /// Shows that `&mut (*&f)` is Ok in Noir, contrary to Rust.
 #[test]
 fn can_reborrow_through_immutable_ref() {
     let src = "
     fn main() {
         let mut f: u64 = 10;

compiler/noirc_frontend/src/hir/comptime/interpreter.rs

@@ -55,7 +55,7 @@ use crate::monomorphization::{
     undo_instantiation_bindings,
 };
 use crate::node_interner::GlobalValue;
-use crate::shared::{ForeignCall, Signedness};
+use crate::shared::{Builtin, ForeignCall, Signedness};
 use crate::token::{FmtStrFragment, Tokens};
 use crate::{
     Shared, Type, TypeBindings,
@@ -330,10 +330,18 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
         let func_attrs = &attributes.function()
             .expect("all builtin functions must contain a function attribute which contains the opcode which it links to").kind;
 
-        if let Some(builtin) = func_attrs.builtin() {
-            self.call_builtin(builtin.clone().as_str(), arguments, return_type, location)
-        } else if let Some(foreign) = func_attrs.foreign() {
-            self.call_foreign(foreign.clone().as_str(), arguments, return_type, location)
+        if let Some(name) = func_attrs.builtin() {
+            let Some(builtin) = Builtin::lookup(name) else {
+                let item = format!("Comptime evaluation for builtin function '{name}'");
+                return Err(InterpreterError::Unimplemented { item, location });
+            };
+            self.call_builtin(builtin, arguments, return_type, location)
+        } else if let Some(name) = func_attrs.foreign() {
+            let Some(foreign) = Builtin::lookup(name) else {
+                let item = format!("Comptime evaluation for foreign function '{name}'");
+                return Err(InterpreterError::Unimplemented { item, location });
+            };
+            self.call_foreign(foreign, arguments, return_type, location)
         } else if let Some(oracle) = func_attrs.oracle() {
             if let Some(ForeignCall::Print) = ForeignCall::lookup(oracle) {
                 self.print_oracle(&arguments)