Refactor operation successor macros

macro/src/dialect/generation.rs

@@ -2,6 +2,7 @@ mod attribute_accessor;
 mod field_accessor;
 mod operation_builder;
 mod region_accessor;
+mod successor_accessor;
 
 use self::{
     attribute_accessor::generate_attribute_accessors,
@@ -10,6 +11,7 @@ use self::{
         generate_default_constructor, generate_operation_builder, generate_operation_builder_fn,
     },
     region_accessor::generate_region_accessor,
+    successor_accessor::generate_successor_accessor,
 };
 use super::operation::{Operation, OperationBuilder};
 use crate::dialect::error::Error;
@@ -26,19 +28,24 @@ pub fn generate_operation(operation: &Operation) -> Result<TokenStream, Error> {
         .general_fields()
         .map(generate_accessor)
         .collect::<Result<Vec<_>, _>>()?;
+    let successor_accessors = operation
+        .successors()
+        .enumerate()
+        .map(|(index, region)| generate_successor_accessor(index, region))
+        .collect::<Vec<_>>();
     let region_accessors = operation
         .regions()
         .enumerate()
         .map(|(index, region)| generate_region_accessor(index, region))
-        .collect::<Result<Vec<_>, _>>()?;
+        .collect::<Vec<_>>();
     let attribute_accessors = operation
         .attributes()
         .map(generate_attribute_accessors)
         .collect::<Result<Vec<_>, _>>()?;
 
     let builder = OperationBuilder::new(operation)?;
     let builder_tokens = generate_operation_builder(&builder)?;
-    let builder_fn = generate_operation_builder_fn(&builder)?;
+    let builder_fn = generate_operation_builder_fn(&builder);
     let default_constructor = generate_default_constructor(&builder)?;
 
     Ok(quote! {
@@ -61,6 +68,7 @@ pub fn generate_operation(operation: &Operation) -> Result<TokenStream, Error> {
             #builder_fn
 
             #(#field_accessors)*
+            #(#successor_accessors)*
             #(#region_accessors)*
             #(#attribute_accessors)*
         }

macro/src/dialect/generation/field_accessor.rs

@@ -129,20 +129,5 @@ fn generate_getter(field: &OperationField) -> TokenStream {
                 }
             }
         }
-        FieldKind::Successor {
-            constraint,
-            sequence_info: SequenceInfo { index, .. },
-        } => {
-            if constraint.is_variadic() {
-                // Only the last successor can be variadic
-                quote! {
-                    self.operation.successors().skip(#index)
-                }
-            } else {
-                quote! {
-                    self.operation.successor(#index)
-                }
-            }
-        }
     }
 }

macro/src/dialect/generation/operation_builder.rs

@@ -28,7 +28,7 @@ pub fn generate_operation_builder(builder: &OperationBuilder) -> Result<TokenStr
         .map(|name| quote! { #name: ::std::marker::PhantomData })
         .collect::<Vec<_>>();
 
-    let builder_fns = generate_field_fns(builder, &field_names, phantom_arguments.as_slice())?;
+    let builder_fns = generate_field_fns(builder, &field_names, phantom_arguments.as_slice());
 
     let new_fn = generate_new_fn(builder, phantom_arguments.as_slice())?;
     let build_fn = generate_build_fn(builder)?;
@@ -57,20 +57,20 @@ fn generate_field_fns(
     builder: &OperationBuilder,
     field_names: &[Ident],
     phantoms: &[TokenStream],
-) -> Result<Vec<TokenStream>, Error> {
+) -> Vec<TokenStream> {
     builder.operation().fields().map(move |field| {
         let builder_identifier = builder.identifier();
-        let identifier = sanitize_snake_case_identifier(field.name())?;
+        let identifier = field.singular_identifier();
         let parameter_type = field.parameter_type();
         let argument = quote! { #identifier: #parameter_type };
         let add = format_ident!("add_{}", field.plural_kind_identifier());
 
         // Argument types can be singular and variadic. But `add` functions in Melior
         // are always variadic, so we need to create a slice or `Vec` for singular
         // arguments.
-        let add_arguments = field.add_arguments(&identifier);
+        let add_arguments = field.add_arguments(identifier);
 
-        Ok(if field.is_optional() {
+        if field.is_optional() {
             let parameters = builder.type_state().parameters().collect::<Vec<_>>();
 
             quote! {
@@ -101,7 +101,7 @@ fn generate_field_fns(
                     }
                 }
             }
-        })
+        }
     }).collect()
 }
 
@@ -145,18 +145,18 @@ fn generate_new_fn(
     })
 }
 
-pub fn generate_operation_builder_fn(builder: &OperationBuilder) -> Result<TokenStream, Error> {
+pub fn generate_operation_builder_fn(builder: &OperationBuilder) -> TokenStream {
     let builder_ident = builder.identifier();
     let arguments = builder.type_state().arguments_all_set(false);
 
-    Ok(quote! {
+    quote! {
         pub fn builder(
             context: &'c ::melior::Context,
             location: ::melior::ir::Location<'c>
-        ) -> #builder_ident<'c, #(#arguments),*> {
+            ) -> #builder_ident<'c, #(#arguments),*> {
             #builder_ident::new(context, location)
         }
-    })
+    }
 }
 
 pub fn generate_default_constructor(builder: &OperationBuilder) -> Result<TokenStream, Error> {

macro/src/dialect/generation/region_accessor.rs

@@ -1,11 +1,8 @@
-use crate::dialect::{
-    error::Error,
-    operation::{OperationFieldLike, Region},
-};
+use crate::dialect::operation::{OperationFieldLike, Region};
 use proc_macro2::TokenStream;
 use quote::quote;
 
-pub fn generate_region_accessor(index: usize, region: &Region) -> Result<TokenStream, Error> {
+pub fn generate_region_accessor(index: usize, region: &Region) -> TokenStream {
     let identifier = &region.singular_identifier();
     let return_type = &region.return_type();
     let body = if region.is_variadic() {
@@ -19,9 +16,9 @@ pub fn generate_region_accessor(index: usize, region: &Region) -> Result<TokenSt
         }
     };
 
-    Ok(quote! {
+    quote! {
         pub fn #identifier(&self, context: &'c ::melior::Context) -> #return_type {
             #body
         }
-    })
+    }
 }

macro/src/dialect/generation/successor_accessor.rs

@@ -0,0 +1,25 @@
+use crate::dialect::operation::{OperationFieldLike, Successor};
+use proc_macro2::TokenStream;
+use quote::quote;
+
+pub fn generate_successor_accessor(index: usize, successor: &Successor) -> TokenStream {
+    let identifier = successor.singular_identifier();
+    let return_type = successor.return_type();
+    let body = if successor.is_variadic() {
+        // Only the last successor can be variadic.
+        quote! {
+            self.operation.successors().skip(#index)
+        }
+    } else {
+        quote! {
+            self.operation.successor(#index)
+        }
+    };
+
+    quote! {
+        #[allow(clippy::needless_question_mark)]
+        pub fn #identifier(&self, context: &'c ::melior::Context) -> #return_type {
+            #body
+        }
+    }
+}

macro/src/dialect/operation.rs

@@ -5,12 +5,13 @@ mod field_kind;
 mod operation_field;
 mod region;
 mod sequence_info;
+mod successor;
 mod variadic_kind;
 
 pub use self::{
     attribute::Attribute, builder::OperationBuilder, element_kind::ElementKind,
     field_kind::FieldKind, operation_field::OperationField, region::Region,
-    sequence_info::SequenceInfo, variadic_kind::VariadicKind,
+    sequence_info::SequenceInfo, successor::Successor, variadic_kind::VariadicKind,
 };
 use super::utility::sanitize_documentation;
 use crate::dialect::{
@@ -26,7 +27,7 @@ pub struct Operation<'a> {
     definition: Record<'a>,
     can_infer_type: bool,
     regions: Vec<Region<'a>>,
-    successors: Vec<OperationField<'a>>,
+    successors: Vec<Successor<'a>>,
     results: Vec<OperationField<'a>>,
     operands: Vec<OperationField<'a>>,
     attributes: Vec<Attribute<'a>>,
@@ -143,17 +144,18 @@ impl<'a> Operation<'a> {
     }
 
     pub fn general_fields(&self) -> impl Iterator<Item = &OperationField<'a>> + Clone {
-        self.results
-            .iter()
-            .chain(&self.operands)
-            .chain(&self.successors)
+        self.results.iter().chain(&self.operands)
     }
 
-    pub fn regions(&self) -> impl Iterator<Item = &Region<'a>> + Clone {
+    pub fn successors(&self) -> impl Iterator<Item = &Successor<'a>> {
+        self.successors.iter()
+    }
+
+    pub fn regions(&self) -> impl Iterator<Item = &Region<'a>> {
         self.regions.iter()
     }
 
-    pub fn attributes(&self) -> impl Iterator<Item = &Attribute<'a>> + Clone {
+    pub fn attributes(&self) -> impl Iterator<Item = &Attribute<'a>> {
         self.attributes.iter().chain(&self.derived_attributes)
     }
 
@@ -162,22 +164,16 @@ impl<'a> Operation<'a> {
             .filter(|field| (!field.is_result() || !self.can_infer_type) && !field.is_optional())
     }
 
-    fn collect_successors(definition: Record<'a>) -> Result<Vec<OperationField>, Error> {
-        let successors_dag = definition.dag_value("successors")?;
-        let len = successors_dag.num_args();
-
-        successors_dag
+    fn collect_successors(definition: Record<'a>) -> Result<Vec<Successor>, Error> {
+        definition
+            .dag_value("successors")?
             .args()
-            .enumerate()
-            .map(|(index, (name, value))| {
-                OperationField::new_successor(
+            .map(|(name, value)| {
+                Successor::new(
                     name,
                     SuccessorConstraint::new(
-                        value
-                            .try_into()
-                            .map_err(|error: tblgen::Error| error.set_location(definition))?,
+                        Record::try_from(value).map_err(|error| error.set_location(definition))?,
                     ),
-                    SequenceInfo { index, len },
                 )
             })
             .collect()

macro/src/dialect/operation/field_kind.rs

@@ -1,6 +1,6 @@
 use super::{element_kind::ElementKind, SequenceInfo, VariadicKind};
 use crate::dialect::{
-    types::{SuccessorConstraint, TypeConstraint},
+    types::TypeConstraint,
     utility::{generate_iterator_type, generate_result_type},
 };
 use syn::{parse_quote, Type};
@@ -13,17 +13,12 @@ pub enum FieldKind<'a> {
         sequence_info: SequenceInfo,
         variadic_kind: VariadicKind,
     },
-    Successor {
-        constraint: SuccessorConstraint<'a>,
-        sequence_info: SequenceInfo,
-    },
 }
 
 impl<'a> FieldKind<'a> {
     pub fn is_optional(&self) -> bool {
         match self {
             Self::Element { constraint, .. } => constraint.is_optional(),
-            Self::Successor { .. } => false,
         }
     }
 
@@ -46,14 +41,6 @@ impl<'a> FieldKind<'a> {
                     base_type
                 }
             }
-            Self::Successor { constraint, .. } => {
-                let r#type: Type = parse_quote!(&::melior::ir::Block<'c>);
-                if constraint.is_variadic() {
-                    parse_quote!(&[#r#type])
-                } else {
-                    r#type
-                }
-            }
         }
     }
 
@@ -82,14 +69,6 @@ impl<'a> FieldKind<'a> {
                     generate_iterator_type(base_type)
                 }
             }
-            Self::Successor { constraint, .. } => {
-                let r#type: Type = parse_quote!(::melior::ir::BlockRef<'c, '_>);
-                if constraint.is_variadic() {
-                    generate_iterator_type(r#type)
-                } else {
-                    generate_result_type(r#type)
-                }
-            }
         }
     }
 }

macro/src/dialect/operation/operation_field.rs

@@ -1,15 +1,14 @@
 use super::{element_kind::ElementKind, field_kind::FieldKind, SequenceInfo, VariadicKind};
 use crate::dialect::{
-    error::Error,
-    types::{SuccessorConstraint, TypeConstraint},
-    utility::sanitize_snake_case_identifier,
+    error::Error, types::TypeConstraint, utility::sanitize_snake_case_identifier,
 };
 use proc_macro2::{Ident, TokenStream};
 use quote::{format_ident, quote};
 use syn::Type;
 
 // TODO Rename this `OperationField`.
 pub trait OperationFieldLike {
+    // TODO Remove this.
     fn name(&self) -> &str;
     fn singular_identifier(&self) -> &Ident;
     fn plural_kind_identifier(&self) -> Ident;
@@ -73,13 +72,6 @@ impl OperationFieldLike for OperationField<'_> {
                     quote! { &[#name] }
                 }
             }
-            FieldKind::Successor { constraint, .. } => {
-                if constraint.is_variadic() {
-                    quote! { #name }
-                } else {
-                    quote! { &[#name] }
-                }
-            }
         }
     }
 }
@@ -90,27 +82,12 @@ impl<'a> OperationField<'a> {
             name,
             plural_identifier: match kind {
                 FieldKind::Element { kind, .. } => format_ident!("{}s", kind.as_str()),
-                FieldKind::Successor { .. } => format_ident!("successors"),
             },
             sanitized_name: sanitize_snake_case_identifier(name)?,
             kind,
         })
     }
 
-    pub fn new_successor(
-        name: &'a str,
-        constraint: SuccessorConstraint<'a>,
-        sequence_info: SequenceInfo,
-    ) -> Result<Self, Error> {
-        Self::new(
-            name,
-            FieldKind::Successor {
-                constraint,
-                sequence_info,
-            },
-        )
-    }
-
     pub fn new_element(
         name: &'a str,
         constraint: TypeConstraint<'a>,