diff --git a/compiler/qsc/src/incremental.rs b/compiler/qsc/src/incremental.rs index 0f422f39b2..7482b616e8 100644 --- a/compiler/qsc/src/incremental.rs +++ b/compiler/qsc/src/incremental.rs @@ -77,27 +77,50 @@ impl Compiler { /// get information about the newly added items, or do other modifications. /// It is then the caller's responsibility to merge /// these packages into the current `CompileUnit` using the `update()` method. - pub fn compile_fragments( + pub fn compile_fragments_fail_fast( &mut self, source_name: &str, source_contents: &str, ) -> Result { - let (core, unit) = self.store.get_open_mut(); - - let mut increment = self - .frontend - .compile_fragments(unit, source_name, source_contents) - .map_err(into_errors)?; + self.compile_fragments(source_name, source_contents, fail_on_error) + } - let pass_errors = self.passes.run_default_passes( - &mut increment.hir, - &mut unit.assigner, - core, - PackageType::Lib, - ); + /// Compiles Q# fragments. See [`compile_fragments_fail_fast`] for more details. + /// + /// This method calls an accumulator function with any errors returned + /// from each of the stages (parsing, lowering). + /// If the accumulator succeeds, compilation continues. + /// If the accumulator returns an error, compilation stops and the + /// error is returned to the caller. + pub fn compile_fragments( + &mut self, + source_name: &str, + source_contents: &str, + mut accumulate_errors: F, + ) -> Result + where + F: FnMut(Errors) -> Result<(), Errors>, + { + let (core, unit) = self.store.get_open_mut(); - if !pass_errors.is_empty() { - return Err(into_errors_with_source(pass_errors, &unit.sources)); + let mut errors = false; + let mut increment = + self.frontend + .compile_fragments(unit, source_name, source_contents, |e| { + errors = errors || !e.is_empty(); + accumulate_errors(into_errors(e)) + })?; + + // Even if we don't fail fast, skip passes if there were compilation errors. + if !errors { + let pass_errors = self.passes.run_default_passes( + &mut increment.hir, + &mut unit.assigner, + core, + PackageType::Lib, + ); + + accumulate_errors(into_errors_with_source(pass_errors, &unit.sources))?; } Ok(increment) @@ -189,3 +212,10 @@ where .map(qsc_frontend::error::WithSource::into_with_source) .collect() } + +fn fail_on_error(errors: Errors) -> Result<(), Errors> { + if !errors.is_empty() { + return Err(errors); + } + Ok(()) +} diff --git a/compiler/qsc/src/interpret/stateful.rs b/compiler/qsc/src/interpret/stateful.rs index cea6d9f66a..273c10c0a3 100644 --- a/compiler/qsc/src/interpret/stateful.rs +++ b/compiler/qsc/src/interpret/stateful.rs @@ -272,7 +272,7 @@ impl Interpreter { let increment = self .compiler - .compile_fragments(&label, fragments) + .compile_fragments_fail_fast(&label, fragments) .map_err(into_errors)?; let stmts = self.lower(&increment); diff --git a/compiler/qsc/src/interpret/stateful/tests.rs b/compiler/qsc/src/interpret/stateful/tests.rs index ec537205b8..8eec5ed6fa 100644 --- a/compiler/qsc/src/interpret/stateful/tests.rs +++ b/compiler/qsc/src/interpret/stateful/tests.rs @@ -750,11 +750,11 @@ mod given_interpreter { &result, &output, &expect![[r#" - name error: `Bar` not found - [line_1] [Bar] - type error: insufficient type information to infer type - [line_1] [Bar()] - "#]], + name error: `Bar` not found + [line_1] [Bar] + type error: insufficient type information to infer type + [line_1] [Bar()] + "#]], ); } @@ -862,11 +862,11 @@ mod given_interpreter { &result, &output, &expect![[r#" - name error: `Bar` not found - [line_2] [Bar] - type error: insufficient type information to infer type - [line_2] [Bar()] - "#]], + name error: `Bar` not found + [line_2] [Bar] + type error: insufficient type information to infer type + [line_2] [Bar()] + "#]], ); } diff --git a/compiler/qsc_frontend/src/incremental.rs b/compiler/qsc_frontend/src/incremental.rs index bda1b0b6ba..23f3d9cd8d 100644 --- a/compiler/qsc_frontend/src/incremental.rs +++ b/compiler/qsc_frontend/src/incremental.rs @@ -95,34 +95,39 @@ impl Compiler { /// get information about the newly added items, or do other modifications. /// It is then the caller's responsibility to merge /// these packages into the current `CompileUnit`. - pub fn compile_fragments( + /// + /// This method calls an accumulator function with any errors returned + /// from each of the stages (parsing, lowering), instead of failing. + /// If the accumulator succeeds, compilation continues. + /// If the accumulator returns an error, compilation stops and the + /// error is returned to the caller. + pub fn compile_fragments( &mut self, unit: &mut CompileUnit, source_name: &str, source_contents: &str, - ) -> Result> { + mut accumulate_errors: F, + ) -> Result + where + F: FnMut(Vec) -> Result<(), E>, + { let (mut ast, parse_errors) = Self::parse_fragments(&mut unit.sources, source_name, source_contents); - if !parse_errors.is_empty() { - return Err(parse_errors); - } + accumulate_errors(parse_errors)?; - let (hir, lower_errors) = self.resolve_check_lower(unit, &mut ast); - - if lower_errors.is_empty() { - Ok(Increment { - ast: AstPackage { - package: ast, - names: self.resolver.names().clone(), - tys: self.checker.table().clone(), - }, - hir, - }) - } else { - self.lowerer.clear_items(); - Err(lower_errors) - } + let (hir, errors) = self.resolve_check_lower(unit, &mut ast); + + accumulate_errors(errors)?; + + Ok(Increment { + ast: AstPackage { + package: ast, + names: self.resolver.names().clone(), + tys: self.checker.table().clone(), + }, + hir, + }) } /// Compiles an entry expression. @@ -148,21 +153,20 @@ impl Compiler { return Err(parse_errors); } - let (package, errors) = self.resolve_check_lower(unit, &mut ast); - - if errors.is_empty() { - Ok(Increment { - ast: AstPackage { - package: ast, - names: self.resolver.names().clone(), - tys: self.checker.table().clone(), - }, - hir: package, - }) - } else { - self.lowerer.clear_items(); - Err(errors) + let (hir, errors) = self.resolve_check_lower(unit, &mut ast); + + if !errors.is_empty() { + return Err(errors); } + + Ok(Increment { + ast: AstPackage { + package: ast, + names: self.resolver.names().clone(), + tys: self.checker.table().clone(), + }, + hir, + }) } pub fn update(&mut self, unit: &mut CompileUnit, new: Increment) { @@ -199,11 +203,26 @@ impl Compiler { let package = self.lower(&mut unit.assigner, &*ast); - let errors: Vec = self + let errors = self + .resolver .drain_errors() - .into_iter() + .map(|e| compile::Error(e.into())) + .chain( + self.checker + .drain_errors() + .map(|e| compile::Error(e.into())), + ) + .chain( + self.lowerer + .drain_errors() + .map(|e| compile::Error(e.into())), + ) .map(|e| WithSource::from_map(&unit.sources, e)) - .collect(); + .collect::>(); + + if !errors.is_empty() { + self.lowerer.clear_items(); + } (package, errors) } @@ -284,23 +303,6 @@ impl Compiler { .with(hir_assigner, self.resolver.names(), self.checker.table()) .lower_package(package) } - - fn drain_errors(&mut self) -> Vec { - self.resolver - .drain_errors() - .map(|e| compile::Error(e.into())) - .chain( - self.checker - .drain_errors() - .map(|e| compile::Error(e.into())), - ) - .chain( - self.lowerer - .drain_errors() - .map(|e| compile::Error(e.into())), - ) - .collect() - } } /// Extends the `Package` with the contents of another `Package`. diff --git a/compiler/qsc_frontend/src/incremental/tests.rs b/compiler/qsc_frontend/src/incremental/tests.rs index 7841cfa6d8..76a746daec 100644 --- a/compiler/qsc_frontend/src/incremental/tests.rs +++ b/compiler/qsc_frontend/src/incremental/tests.rs @@ -2,7 +2,10 @@ // Licensed under the MIT License. use super::{Compiler, Increment}; -use crate::compile::{self, CompileUnit, PackageStore, TargetProfile}; +use crate::{ + compile::{self, CompileUnit, PackageStore, TargetProfile}, + incremental::Error, +}; use expect_test::{expect, Expect}; use indoc::indoc; use miette::Diagnostic; @@ -17,6 +20,7 @@ fn one_callable() { &mut CompileUnit::default(), "test_1", "namespace Foo { operation Main() : Unit {} }", + fail_on_error, ) .expect("compilation should succeed"); @@ -60,7 +64,12 @@ fn one_statement() { let store = PackageStore::new(compile::core()); let mut compiler = Compiler::new(&store, vec![], TargetProfile::Full); let unit = compiler - .compile_fragments(&mut CompileUnit::default(), "test_1", "use q = Qubit();") + .compile_fragments( + &mut CompileUnit::default(), + "test_1", + "use q = Qubit();", + fail_on_error, + ) .expect("compilation should succeed"); check_unit( @@ -89,10 +98,39 @@ fn parse_error() { let store = PackageStore::new(compile::core()); let mut compiler = Compiler::new(&store, vec![], TargetProfile::Full); let errors = compiler - .compile_fragments(&mut CompileUnit::default(), "test_1", "}}") + .compile_fragments(&mut CompileUnit::default(), "test_1", "}}", fail_on_error) .expect_err("should fail"); - assert!(!errors.is_empty()); + expect![[r#" + [ + WithSource { + sources: [ + Source { + name: "test_1", + contents: "}}", + offset: 0, + }, + ], + error: Error( + Parse( + Error( + Token( + Eof, + Close( + Brace, + ), + Span { + lo: 0, + hi: 1, + }, + ), + ), + ), + ), + }, + ] + "#]] + .assert_debug_eq(&errors); } #[test] @@ -111,6 +149,7 @@ fn conditional_compilation_not_available() { Dropped(); } "}, + fail_on_error, ) .expect_err("should fail"); @@ -129,19 +168,25 @@ fn errors_across_multiple_lines() { &mut unit, "line_1", "namespace Other { operation DumpMachine() : Unit { } }", + fail_on_error, ) .expect("should succeed"); compiler - .compile_fragments(&mut unit, "line_2", "open Other;") + .compile_fragments(&mut unit, "line_2", "open Other;", fail_on_error) .expect("should succeed"); compiler - .compile_fragments(&mut unit, "line_3", "open Microsoft.Quantum.Diagnostics;") + .compile_fragments( + &mut unit, + "line_3", + "open Microsoft.Quantum.Diagnostics;", + fail_on_error, + ) .expect("should succeed"); let errors = compiler - .compile_fragments(&mut unit, "line_4", "DumpMachine()") + .compile_fragments(&mut unit, "line_4", "DumpMachine()", fail_on_error) .expect_err("should fail"); // Here we're validating that the compiler is able to return @@ -177,6 +222,124 @@ fn errors_across_multiple_lines() { .assert_debug_eq(&labels); } +#[test] +fn continue_after_parse_error() { + let store = PackageStore::new(compile::core()); + let mut compiler = Compiler::new(&store, vec![], TargetProfile::Full); + let mut errors = Vec::new(); + + compiler + .compile_fragments( + &mut CompileUnit::default(), + "test_1", + "operation Main() : Foo { + }}", + |e| -> Result<(), ()> { + errors.extend(e); + Ok(()) + }, + ) + .expect("compile_fragments should succeed"); + + expect![[r#" + [ + WithSource { + sources: [ + Source { + name: "test_1", + contents: "operation Main() : Foo {\n }}", + offset: 0, + }, + ], + error: Error( + Parse( + Error( + Token( + Eof, + Close( + Brace, + ), + Span { + lo: 38, + hi: 39, + }, + ), + ), + ), + ), + }, + WithSource { + sources: [ + Source { + name: "test_1", + contents: "operation Main() : Foo {\n }}", + offset: 0, + }, + ], + error: Error( + Resolve( + NotFound( + "Foo", + Span { + lo: 19, + hi: 22, + }, + ), + ), + ), + }, + ] + "#]] + .assert_debug_eq(&errors); +} + +#[test] +fn continue_after_lower_error() { + let store = PackageStore::new(compile::core()); + let mut compiler = Compiler::new(&store, vec![], TargetProfile::Full); + let mut unit = CompileUnit::default(); + + let mut errors = Vec::new(); + + compiler + .compile_fragments( + &mut unit, + "test_1", + "operation A(q : Qubit) : Unit is Adj { + adjoint ... {} + }", + |e| -> Result<(), ()> { + errors = e; + Ok(()) + }, + ) + .expect("compile_fragments should succeed"); + + expect![[r#" + [ + WithSource { + sources: [ + Source { + name: "test_1", + contents: "operation A(q : Qubit) : Unit is Adj {\n adjoint ... {}\n }", + offset: 0, + }, + ], + error: Error( + Lower( + MissingBody( + Span { + lo: 0, + hi: 83, + }, + ), + ), + ), + }, + ] + "#]].assert_debug_eq(&errors); +} + fn check_unit(expect: &Expect, actual: &Increment) { let ast = format!("ast:\n{}", actual.ast.package); @@ -208,3 +371,10 @@ fn check_unit(expect: &Expect, actual: &Increment) { expect.assert_eq(&[ast, names, terms, hir].into_iter().collect::()); } + +fn fail_on_error(errors: Vec) -> Result<(), Vec> { + if !errors.is_empty() { + return Err(errors); + } + Ok(()) +}