Add map of imported procedures to AST structs

[jjcnn]

Describe your changes

A map from procedure IDs of imported procedures to the procedure's name and module path has been added to ProgramAst and ModuleAst.

This map is needed for the MASM pretty-printer, because the instructions that represent calls to imported procedures only contain the procedure's ID and not its name. The procedure's ID is a hash, and it is therefore impossible to print the assembly for the call instruction without this map.

The map is populated during parsing as part of the ParserContext. Since the parser does not have access to the code of imported modules, only the procedures that are actually called are represented in the map. This also explains why the map is named used_imported_procs.

Like the imports map, this new map can optionally be serialized when the AST is serialized. Serialization and deserialization tests have been added.

The only user-facing change is the new limitation of max 2^16-1 calls to imported procedures in each program and module. We have not added documentation for similar restrictions in the past, so I haven't added anything this time either.

Checklist before requesting a review

• Repo forked and branch created from next according to naming convention.
• Commit messages and codestyle follow conventions.
• Relevant issues are linked in the PR description.
• Tests added for new functionality.
• Documentation/comments updated according to changes.

[tohrnii]

Thank you. Looks good to me.

[bobbinth]

Looks good! Thank you! I left a small comment inline, but I also do wonder about one thing: is there a use-case when we'd want imports but not used imported procs and vice-versa? Or do they always go together?

My thinking was that they always go together and we can view them kind of like extra information about imports used within a module. So, maybe a better approach is to create a dedicate struct for this? This struct could look something like this:

pub struct ModuleImports {
    imported_modules: BTreeMap<String, LibraryPath>,
    used_procs: BTreeMap<ProcedureId, (ProcedureName, String)>,
}

This might be a good way to encapsulate this info, and we'd be able to add various convenience methods to this as well. For example:

impl ModuleImports {
    pub fn get_module_path(&self, module_name: &str) -> Option<LibraryPath> {
        todo!()
    }

    pub fn add_used_procedure(&mut self, proc_name: ProcedureName, module_name: &str) {
        todo!()
    }
 
    pub fn get_unused_imports(&self) -> &[LibraryPath] {
        todo!()
    }
}

And this struct could also be serialized (or not) in its entirety.

[jjcnn]

is there a use-case when we'd want imports but not used imported procs and vice-versa? Or do they always go together?

I'm very unsure about the use cases beyond pretty-printing. For pretty-printing we need both, and I can't imagine you would ever need the procedure names map without also needing the import paths, so there's definitely a dependency there.

I'll stick it all into a dedicated struct for now. We can always separate them out later if we need to.

[jjcnn]

Review comments addressed. The resulting refactoring is quite extensive, but it probably makes better sense this way.

[bobbinth]

Looks good! Thank you! I left some inline comments. The biggest thing is that I think we could move more logic into the new ModuleImports struct - specifically parsing and validation logic. If we do the latter, we can also insure that ModuleImports is always in a valid state.

[bobbinth]

Looks good! Thank you! I left a couple more small comments. Also, take a look at my last commit to see if I missed anything there.

[jjcnn]

Review comments fixed, and branch rebased to next.

[bobbinth]

Looks good! Thank you! I left a couple of doc-related comments inline. Once these are addressed, we can merge.