fix(assembly): address conflicting procedure definitions bug

Previously, we assumed that two procedures with the same MAST root, but
differing numbers of locals, was a bug in the assembler. However, this
is not the case, as I will elaborate on below.

If you compile a program like so:

```masm
use.std::u64

begin
  exec.u64::checked_and
end
```

The resulting MAST would look something like:

```mast
begin
  external.0x....
end
```

This MAST will have the exact same MAST root as `std::u64::checked_and`,
because `external` nodes have the same digest as the node they refer to.
Now, if the exec'd procedure has the same number of locals as the
caller, this is presumed to be a "compatible" procedure, meaning it is
fine to let both procedures refer to the same MAST. However, when the
number of procedure locals _differs_, we were raising a compatible
definition error, because it was assumed that due to the instructions
added when procedure locals are present, two procedures with differing
numbers of locals _could not_ have the same root by definition. This is
incorrect, let me illustrate:

```masm

export.foo.2
  ...
end

use.foo

proc.bar.3
   exec.foo::foo
end

begin
   exec.foo::foo
end
```

Assume that `foo.masm` is first compiled to a `CompiledLibrary`, which
is then added to the assembler when assembling an executable program
from `bar.masm`.

Also, recall that `begin .. end` blocks are effectively an exported
procedure definition, with zero locals, that has a special name - but in
all other ways it is just a normal procedure.

The above program is perfectly legal, but we would raise an error during
assembly of the program due to the `begin .. end` block compiling to an
`external` node referencing the MAST root of `foo::foo`, which has a
non-zero number of procedure locals.

The `bar` procedure is there to illustrate that even though it too
simply "wraps" the `foo::foo` procedure, it has a non-zero number of
procedure locals, and thus cannot ever have the same MAST root as a
wrapped procedure with a non-zero number of locals, due to the presence
of locals changing the root of the wrapping procedure. A check has been
kept around that ensures we catch if ever there are two procedures with
non-zero counts of procedure locals, with the same MAST root.

assembly/src/assembler/mast_forest_builder.rs

@@ -95,17 +95,9 @@ impl MastForestBuilder {
         //
         // If there is already a cache entry, but it conflicts with what we're trying to cache,
         // then raise an error.
-        if let Some(cached) = self.procedures.get(&gid) {
-            let cached_root = self.mast_forest[cached.body_node_id()].digest();
-            if cached_root != proc_root || cached.num_locals() != procedure.num_locals() {
-                return Err(AssemblyError::ConflictingDefinitions {
-                    first: cached.fully_qualified_name().clone(),
-                    second: procedure.fully_qualified_name().clone(),
-                });
-            }
-
+        if self.procedures.contains_key(&gid) {
             // The global procedure index and the MAST root resolve to an already cached version of
-            // this procedure, nothing to do.
+            // this procedure, or an alias of it, nothing to do.
             //
             // TODO: We should emit a warning for this, because while it is not an error per se, it
             // does reflect that we're doing work we don't need to be doing. However, emitting a
@@ -117,20 +109,24 @@ impl MastForestBuilder {
         // We don't have a cache entry yet, but we do want to make sure we don't have a conflicting
         // cache entry with the same MAST root:
         if let Some(cached) = self.find_procedure(&proc_root) {
-            if cached.num_locals() != procedure.num_locals() {
+            // Handle the case where a procedure with no locals is lowered to a MastForest
+            // consisting only of an `External` node to another procedure which has one or more
+            // locals. This will result in the calling procedure having the same digest as the
+            // callee, but the two procedures having mismatched local counts. When this occurs,
+            // we want to use the procedure with non-zero local count as the definition, and treat
+            // the other procedure as an alias, which can be referenced like any other procedure,
+            // but the MAST returned for it will be that of the "real" definition.
+            let cached_locals = cached.num_locals();
+            let procedure_locals = procedure.num_locals();
+            let mismatched_locals = cached_locals != procedure_locals;
+            let is_valid =
+                !mismatched_locals || core::cmp::min(cached_locals, procedure_locals) == 0;
+            if !is_valid {
                 return Err(AssemblyError::ConflictingDefinitions {
                     first: cached.fully_qualified_name().clone(),
                     second: procedure.fully_qualified_name().clone(),
                 });
             }
-
-            // We have a previously cached version of an equivalent procedure, just under a
-            // different [GlobalProcedureIndex], so insert the cached procedure into the slot for
-            // `id`, but skip inserting a record in the MAST root lookup table
-            self.make_root(procedure.body_node_id());
-            self.insert_procedure_hash(gid, procedure.mast_root())?;
-            self.procedures.insert(gid, Arc::new(procedure));
-            return Ok(());
         }
 
         self.make_root(procedure.body_node_id());

assembly/src/assembler/module_graph/mod.rs

@@ -549,43 +549,6 @@ impl ModuleGraph {
             Entry::Occupied(ref mut entry) => {
                 let prev_id = entry.get()[0];
                 if prev_id != id {
-                    let prev_proc = {
-                        match &self.modules[prev_id.module.as_usize()] {
-                            WrappedModule::Ast(module) => Some(&module[prev_id.index]),
-                            WrappedModule::Info(_) => None,
-                        }
-                    };
-                    let current_proc = {
-                        match &self.modules[id.module.as_usize()] {
-                            WrappedModule::Ast(module) => Some(&module[id.index]),
-                            WrappedModule::Info(_) => None,
-                        }
-                    };
-
-                    // Note: For compiled procedures, we can't check further if they're compatible,
-                    // so we assume they are.
-                    if let (Some(prev_proc), Some(current_proc)) = (prev_proc, current_proc) {
-                        if prev_proc.num_locals() != current_proc.num_locals() {
-                            // Multiple procedures with the same root, but incompatible
-                            let prev_module = self.modules[prev_id.module.as_usize()].path();
-                            let prev_name = QualifiedProcedureName {
-                                span: prev_proc.span(),
-                                module: prev_module.clone(),
-                                name: prev_proc.name().clone(),
-                            };
-                            let current_module = self.modules[id.module.as_usize()].path();
-                            let current_name = QualifiedProcedureName {
-                                span: current_proc.span(),
-                                module: current_module.clone(),
-                                name: current_proc.name().clone(),
-                            };
-                            return Err(AssemblyError::ConflictingDefinitions {
-                                first: prev_name,
-                                second: current_name,
-                            });
-                        }
-                    }
-
                     // Multiple procedures with the same root, but compatible
                     entry.get_mut().push(id);
                 }