Use dense FIR IDs across packages (#1296)

This change fixes three things:
- It reduces the memory footprint by using dense FIR IDs across
packages. This is accomplished by using a new lowerer instance for every
package such that IDs for blocks, statements, expressions and pats are
reset and start from zero.
- It fixes a bug where the debugger would not honor a breakpoint if the
statement that mapped to the breakpoint had the ID 0.
- As a consequence of changing the way the FIR lowerer is used, the
debugger would now randomly and inadvertently break into a core or
standard library statement if the statement ID happened to be the same
than the statement ID where a breakpoint is set in the user's code. This
problem is also addressed by this change.

This was done in collaboration with @swernli and @idavis.

---------

Co-authored-by: Stefan J. Wernli <[email protected]>

Author: cesarzc

compiler/qsc/benches/rca.rs

@@ -134,26 +134,22 @@ impl Default for CompilationContext {
             LanguageFeatures::default(),
         )
         .expect("should be able to create a new compiler");
-        let mut lowerer = Lowerer::new();
-        let fir_store = lower_hir_package_store(&mut lowerer, compiler.package_store());
+        let fir_store = lower_hir_package_store(compiler.package_store());
         Self {
             compiler,
-            lowerer,
+            lowerer: Lowerer::new(),
             fir_store,
             compute_properties: None,
         }
     }
 }
 
-fn lower_hir_package_store(
-    lowerer: &mut Lowerer,
-    hir_package_store: &HirPackageStore,
-) -> PackageStore {
+fn lower_hir_package_store(hir_package_store: &HirPackageStore) -> PackageStore {
     let mut fir_store = PackageStore::new();
     for (id, unit) in hir_package_store {
         fir_store.insert(
             map_hir_package_to_fir(id),
-            lowerer.lower_package(&unit.package),
+            Lowerer::new().lower_package(&unit.package),
         );
     }
     fir_store

compiler/qsc/src/interpret.rs

@@ -145,28 +145,25 @@ impl Interpreter {
         capabilities: RuntimeCapabilityFlags,
         language_features: LanguageFeatures,
     ) -> std::result::Result<Self, Vec<Error>> {
-        let mut lowerer = qsc_eval::lower::Lowerer::new();
-        let mut fir_store = fir::PackageStore::new();
-
         let compiler = Compiler::new(std, sources, package_type, capabilities, language_features)
             .map_err(into_errors)?;
 
+        let mut fir_store = fir::PackageStore::new();
         for (id, unit) in compiler.package_store() {
             fir_store.insert(
                 map_hir_package_to_fir(id),
-                lowerer.lower_package(&unit.package),
+                qsc_eval::lower::Lowerer::new().lower_package(&unit.package),
             );
         }
 
         let source_package_id = compiler.source_package_id();
         let package_id = compiler.package_id();
-
         Ok(Self {
             compiler,
             lines: 0,
             capabilities,
             fir_store,
-            lowerer,
+            lowerer: qsc_eval::lower::Lowerer::new(),
             env: Env::default(),
             sim: BackendChain::new(
                 SparseSim::new(),

compiler/qsc_codegen/src/qir_base.rs

@@ -31,12 +31,11 @@ pub fn generate_qir(
     store: &PackageStore,
     package: hir::PackageId,
 ) -> std::result::Result<String, (Error, Vec<Frame>)> {
-    let mut fir_lowerer = qsc_eval::lower::Lowerer::new();
     let mut fir_store = fir::PackageStore::new();
     for (id, unit) in store {
         fir_store.insert(
             map_hir_package_to_fir(id),
-            fir_lowerer.lower_package(&unit.package),
+            qsc_eval::lower::Lowerer::new().lower_package(&unit.package),
         );
     }
 

compiler/qsc_eval/src/intrinsic/tests.rs

@@ -146,10 +146,9 @@ impl Backend for CustomSim {
 }
 
 fn check_intrinsic(file: &str, expr: &str, out: &mut impl Receiver) -> Result<Value, Error> {
-    let mut fir_lowerer = crate::lower::Lowerer::new();
     let mut core = compile::core();
     run_core_passes(&mut core);
-    let core_fir = fir_lowerer.lower_package(&core.package);
+    let core_fir = crate::lower::Lowerer::new().lower_package(&core.package);
     let mut store = PackageStore::new(core);
 
     let mut std = compile::std(&store, RuntimeCapabilityFlags::all());
@@ -161,7 +160,7 @@ fn check_intrinsic(file: &str, expr: &str, out: &mut impl Receiver) -> Result<Va
         RuntimeCapabilityFlags::all()
     )
     .is_empty());
-    let std_fir = fir_lowerer.lower_package(&std.package);
+    let std_fir = crate::lower::Lowerer::new().lower_package(&std.package);
     let std_id = store.insert(std);
 
     let sources = SourceMap::new([("test".into(), file.into())], Some(expr.into()));
@@ -180,7 +179,7 @@ fn check_intrinsic(file: &str, expr: &str, out: &mut impl Receiver) -> Result<Va
         RuntimeCapabilityFlags::all()
     )
     .is_empty());
-    let unit_fir = fir_lowerer.lower_package(&unit.package);
+    let unit_fir = crate::lower::Lowerer::new().lower_package(&unit.package);
     let entry = unit_fir.entry_exec_graph.clone();
 
     let id = store.insert(unit);

compiler/qsc_eval/src/lib.rs

@@ -406,6 +406,7 @@ pub struct State {
     idx_stack: Vec<u32>,
     val_register: Option<Value>,
     val_stack: Vec<Vec<Value>>,
+    source_package: PackageId,
     package: PackageId,
     call_stack: CallStack,
     current_span: Span,
@@ -429,6 +430,7 @@ impl State {
             idx_stack: Vec::new(),
             val_register: None,
             val_stack: vec![Vec::new()],
+            source_package: package,
             package,
             call_stack: CallStack::default(),
             current_span: Span::default(),
@@ -547,7 +549,10 @@ impl State {
                     self.idx += 1;
                     self.current_span = globals.get_stmt((self.package, *stmt).into()).span;
 
-                    if let Some(bp) = breakpoints.iter().find(|&bp| *bp == *stmt) {
+                    if let Some(bp) = breakpoints
+                        .iter()
+                        .find(|&bp| *bp == *stmt && self.package == self.source_package)
+                    {
                         StepResult::BreakpointHit(*bp)
                     } else {
                         if self.current_span == Span::default() {