simplify port representation

Author: rachitnigam

crates/ast/src/control.rs

@@ -4,7 +4,7 @@ use super::{
 use fil_utils::PortAttrs;
 use struct_variant::struct_variant;
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 /// Access into a bundle
 pub struct Access {
     pub start: Expr,
@@ -33,76 +33,128 @@ impl From<Expr> for Access {
     }
 }
 
-/// A port mentioned in the program
-// XXX(rachit): the bundle and non-bundle variants can be unified because
-// astconv treats them the same anyways.
-#[derive(Clone)]
-pub enum Port {
-    /// A port on this component
-    This(Loc<Id>),
-    /// A port on an invoke
-    InvPort { invoke: Loc<Id>, name: Loc<Id> },
-    /// A port represented by an index into a bundle
-    Bundle {
-        name: Loc<Id>,
-        access: Vec<Loc<Access>>,
-    },
-    /// A bundle port on an invocation
-    InvBundle {
-        invoke: Loc<Id>,
-        port: Loc<Id>,
-        access: Vec<Loc<Access>>,
-    },
+impl std::fmt::Display for Access {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        // For now, always show as range. Could add logic to detect single indices.
+        write!(f, "{}:{}", self.start, self.end)
+    }
+}
+
+/// Unified port representation
+#[derive(Clone, Debug)]
+pub struct Port {
+    /// Base port reference
+    pub base: PortRef,
+    /// Optional array/bundle access
+    pub access: Vec<Loc<Access>>,
+    /// Source location for error reporting
+    pub loc: fil_utils::GPosIdx,
+}
+
+/// Base port reference
+#[derive(Clone, Debug, PartialEq)]
+pub enum PortRef {
+    /// Reference to a port on 'this' component
+    This { port: Loc<Id> },
+    /// Reference to a port on an instance or invoke
+    Instance { instance: Loc<Id>, port: Loc<Id> },
 }
 
 impl Port {
+    /// Create a simple port reference (this.port)
+    pub fn this_port(port: Loc<Id>) -> Self {
+        let loc = port.pos();
+        Port {
+            base: PortRef::This { port },
+            access: Vec::new(),
+            loc,
+        }
+    }
+
+    /// Create an instance port reference (inst.port)
+    pub fn instance_port(instance: Loc<Id>, port: Loc<Id>) -> Self {
+        let loc = instance.pos(); // Using first location, could union if needed
+        Port {
+            base: PortRef::Instance { instance, port },
+            access: Vec::new(),
+            loc,
+        }
+    }
+
+    /// Add array/bundle access to a port
+    pub fn with_access(mut self, access: Vec<Loc<Access>>) -> Self {
+        self.access = access;
+        self
+    }
+
+    /// Check if this is a bundle access
+    pub fn is_bundle(&self) -> bool {
+        !self.access.is_empty()
+    }
+
+    /// Get the instance name if this is an instance port
+    pub fn instance(&self) -> Option<&Loc<Id>> {
+        match &self.base {
+            PortRef::Instance { instance, .. } => Some(instance),
+            _ => None,
+        }
+    }
+
+    /// Get the port name
+    pub fn port_name(&self) -> &Loc<Id> {
+        match &self.base {
+            PortRef::This { port } => port,
+            PortRef::Instance { port, .. } => port,
+        }
+    }
+
+    // Legacy constructors for backwards compatibility during migration
     pub fn inv_port(comp: Loc<Id>, name: Loc<Id>) -> Self {
-        Port::InvPort { invoke: comp, name }
+        Self::instance_port(comp, name)
     }
 
     pub fn this(p: Loc<Id>) -> Self {
-        Port::This(p)
+        Self::this_port(p)
     }
 
     pub fn bundle(name: Loc<Id>, access: Vec<Loc<Access>>) -> Self {
-        Port::Bundle { name, access }
+        Self::this_port(name).with_access(access)
     }
 
     pub fn inv_bundle(
         invoke: Loc<Id>,
         port: Loc<Id>,
         access: Vec<Loc<Access>>,
     ) -> Self {
-        Port::InvBundle {
-            invoke,
-            port,
-            access,
+        Self::instance_port(invoke, port).with_access(access)
+    }
+
+    pub fn resolve_exprs(mut self, bindings: &Binding<Expr>) -> Self {
+        if !self.access.is_empty() {
+            self.access = self
+                .access
+                .into_iter()
+                .map(|i| i.map(|i| i.resolve(bindings)))
+                .collect();
         }
+        self
     }
+}
 
-    pub fn resolve_exprs(self, bindings: &Binding<Expr>) -> Self {
-        match self {
-            Port::Bundle { name, access } => Port::Bundle {
-                name,
-                access: access
-                    .into_iter()
-                    .map(|i| i.map(|i| i.resolve(bindings)))
-                    .collect(),
-            },
-            Port::InvBundle {
-                invoke,
-                port,
-                access,
-            } => Port::InvBundle {
-                invoke,
-                port,
-                access: access
-                    .into_iter()
-                    .map(|i| i.map(|a| a.resolve(bindings)))
-                    .collect(),
-            },
-            _ => self,
+impl std::fmt::Display for Port {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match &self.base {
+            PortRef::This { port } => write!(f, "{}", port),
+            PortRef::Instance { instance, port } => {
+                write!(f, "{}.{}", instance, port)
+            }
+        }?;
+
+        for access in &self.access {
+            write!(f, "{{{}}}", access)?;
         }
+
+        Ok(())
     }
 }
 

crates/ast/src/lib.rs

@@ -16,7 +16,7 @@ pub use component::{Component, Extern, Namespace};
 pub use constraint::{Constraint, OrderConstraint, OrderOp};
 pub use control::{
     Access, Bundle, BundleType, Command, Connect, Exists, Fact, ForLoop, If,
-    Instance, Invoke, ParamLet, Port,
+    Instance, Invoke, ParamLet, Port, PortRef,
 };
 pub use expr::{Expr, Fn, Op};
 pub use fil_utils::Id;

crates/filament/src/main.rs

@@ -5,7 +5,8 @@ use fil_ir as ir;
 use filament::ir_passes::BuildDomination;
 use filament::{ast_pass_pipeline, ir_pass_pipeline, log_pass, log_time};
 use filament::{
-    ast_passes as ap, ast_visitor::Visitor as AstVisitor, cmdline, ir_passes as ip, ir_visitor::Visitor as IrVisitor, resolver::Resolver,
+    ast_passes as ap, ast_visitor::Visitor as AstVisitor, cmdline,
+    ir_passes as ip, ir_visitor::Visitor as IrVisitor, resolver::Resolver,
 };
 use serde::Deserialize;
 use std::collections::HashMap;

crates/ir/src/from_ast/astconv.rs

@@ -517,54 +517,62 @@ impl BuildCtx<'_> {
         port: ast::Port,
         dir: ir::Direction,
     ) -> BuildRes<ir::Access> {
-        let acc = match port {
-            ast::Port::This(n) => {
+        let acc = match (&port.base, &port.access[..]) {
+            (ast::PortRef::This { port: name }, []) => {
                 // NOTE: The AST does not distinguish between ports
                 // defined by the signature and locally defined ports so we
                 // must search both.
-                let owner = OwnedPort::Sig(dir, n.clone());
+                let owner = OwnedPort::Sig(dir, name.clone());
                 let port = if let Some(port) = self.find_port(&owner) {
                     port
                 } else {
-                    let owner = OwnedPort::Local(n);
+                    let owner = OwnedPort::Local(name.clone());
                     self.get_port(&owner)?
                 };
 
                 ir::Access::port(port, self.comp())
             }
-            ast::Port::InvPort { invoke, name } => {
+            (
+                ast::PortRef::Instance {
+                    instance: invoke,
+                    port: name,
+                },
+                [],
+            ) => {
                 let inv = self.get_inv(&invoke)?;
-                let owner = OwnedPort::Inv(inv, dir, name);
+                let owner = OwnedPort::Inv(inv, dir, name.clone());
                 ir::Access::port(self.get_port(&owner)?, self.comp())
             }
-            ast::Port::Bundle { name, access } => {
+            (ast::PortRef::This { port: name }, access) => {
                 // NOTE(rachit): The AST does not distinguish between bundles
                 // defined by the signature and locally defined bundles so we
                 // must search both.
                 let owner = OwnedPort::Sig(dir, name.clone());
                 let port = if let Some(p) = self.find_port(&owner) {
                     p
                 } else {
-                    let owner = OwnedPort::Local(name);
+                    let owner = OwnedPort::Local(name.clone());
                     self.get_port(&owner)?
                 };
                 let ranges = access
-                    .into_iter()
-                    .map(|a| self.access(a.take()))
+                    .iter()
+                    .map(|a| self.access(a.clone().take()))
                     .collect::<BuildRes<Vec<_>>>()?;
                 ir::Access { port, ranges }
             }
-            ast::Port::InvBundle {
-                invoke,
-                port,
+            (
+                ast::PortRef::Instance {
+                    instance: invoke,
+                    port,
+                },
                 access,
-            } => {
+            ) => {
                 let inv = self.get_inv(&invoke)?;
-                let owner = OwnedPort::Inv(inv, dir, port);
+                let owner = OwnedPort::Inv(inv, dir, port.clone());
                 let port = self.get_port(&owner)?;
                 let ranges = access
-                    .into_iter()
-                    .map(|a| self.access(a.take()))
+                    .iter()
+                    .map(|a| self.access(a.clone().take()))
                     .collect::<BuildRes<Vec<_>>>()?;
                 ir::Access { port, ranges }
             }