Skip to content

Commit

Permalink
Improve error reporting spans in RCA check (#1543)
Browse files Browse the repository at this point in the history
This fixes minor issues in error spans used when reporting RCA check
errors to avoid duplicate errors being reported for generated entry
expressions that come from `@EntryPoint` attributed callables in
standalone Q# files.

Fixes #1537.
  • Loading branch information
swernli authored May 20, 2024
1 parent bd19ca1 commit 5a60039
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 51 deletions.
14 changes: 13 additions & 1 deletion compiler/qsc_partial_eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ impl<'a> PartialEvaluator<'a> {
ret_val,
&self.get_expr(self.entry.expr.expr).ty,
)
.map_err(|()| Error::OutputResultLiteral(self.get_expr(self.entry.expr.expr).span))?;
.map_err(|()| Error::OutputResultLiteral(self.entry_expr_output_span()))?;

// Insert the return expression and return the generated program.
let current_block = self.get_current_rir_block_mut();
Expand All @@ -294,6 +294,18 @@ impl<'a> PartialEvaluator<'a> {
Ok(self.program)
}

fn entry_expr_output_span(&self) -> Span {
let expr = self.get_expr(self.entry.expr.expr);
match &expr.kind {
// Special handling for compiler generated entry expressions that come from the `@EntryPoint`
// attributed callable.
ExprKind::Call(callee, _) if expr.span == Span::default() => {
self.get_expr(*callee).span
}
_ => expr.span,
}
}

fn eval_bin_op(
&mut self,
bin_op: BinOp,
Expand Down
42 changes: 25 additions & 17 deletions compiler/qsc_partial_eval/tests/output_recording.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use expect_test::expect;
use indoc::indoc;
use test_utils::get_rir_program;
use test_utils::{assert_error, get_partial_evaluation_error, get_rir_program};

pub mod test_utils;

Expand Down Expand Up @@ -519,11 +519,8 @@ fn output_recording_for_mix_of_literal_and_variable() {
}

#[test]
#[should_panic(
expected = "partial evaluation failed: OutputResultLiteral(Span { lo: 50, hi: 54 })"
)]
fn output_recording_fails_with_result_literal_one() {
let _ = get_rir_program(indoc! {
let error = get_partial_evaluation_error(indoc! {
r#"
namespace Test {
@EntryPoint()
Expand All @@ -533,14 +530,16 @@ fn output_recording_fails_with_result_literal_one() {
}
"#,
});

assert_error(
&error,
&expect!["OutputResultLiteral(Span { lo: 50, hi: 54 })"],
);
}

#[test]
#[should_panic(
expected = "partial evaluation failed: OutputResultLiteral(Span { lo: 50, hi: 54 })"
)]
fn output_recording_fails_with_result_literal_zero() {
let _ = get_rir_program(indoc! {
let error = get_partial_evaluation_error(indoc! {
r#"
namespace Test {
@EntryPoint()
Expand All @@ -550,14 +549,16 @@ fn output_recording_fails_with_result_literal_zero() {
}
"#,
});

assert_error(
&error,
&expect!["OutputResultLiteral(Span { lo: 50, hi: 54 })"],
);
}

#[test]
#[should_panic(
expected = "partial evaluation failed: OutputResultLiteral(Span { lo: 50, hi: 54 })"
)]
fn output_recording_fails_with_result_literal_in_array() {
let _ = get_rir_program(indoc! {
let error = get_partial_evaluation_error(indoc! {
r#"
namespace Test {
@EntryPoint()
Expand All @@ -568,14 +569,16 @@ fn output_recording_fails_with_result_literal_in_array() {
}
"#,
});

assert_error(
&error,
&expect!["OutputResultLiteral(Span { lo: 50, hi: 54 })"],
);
}

#[test]
#[should_panic(
expected = "partial evaluation failed: OutputResultLiteral(Span { lo: 50, hi: 54 })"
)]
fn output_recording_fails_with_result_literal_in_tuple() {
let _ = get_rir_program(indoc! {
let error = get_partial_evaluation_error(indoc! {
r#"
namespace Test {
@EntryPoint()
Expand All @@ -586,4 +589,9 @@ fn output_recording_fails_with_result_literal_in_tuple() {
}
"#,
});

assert_error(
&error,
&expect!["OutputResultLiteral(Span { lo: 50, hi: 54 })"],
);
}
47 changes: 47 additions & 0 deletions compiler/qsc_passes/src/capabilitiesck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,16 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.package.get_stmt(id)
}

fn visit_package(&mut self, package: &'a Package) {
package
.items
.iter()
.for_each(|(_, item)| self.visit_item(item));
package.entry.iter().for_each(|entry_expr_id| {
self.check_entry_expr(*entry_expr_id);
});
}

fn visit_callable_impl(&mut self, callable_impl: &'a CallableImpl) {
match callable_impl {
CallableImpl::Intrinsic => self.check_spec_decl(FunctorSetValue::Empty, None),
Expand Down Expand Up @@ -184,6 +194,16 @@ impl<'a> Checker<'a> {
self.generate_errors()
}

fn check_entry_expr(&mut self, expr_id: ExprId) {
let expr = self.get_expr(expr_id);
if expr.span == Span::default() {
// This is an auto-generated entry expression, so we only need to verify the output recording flags.
self.check_output_recording(expr);
} else {
self.visit_expr(expr_id);
}
}

fn check_expr(&mut self, expr_id: ExprId) {
let compute_kind = self.compute_properties.get_expr(expr_id).inherent;
let ComputeKind::Quantum(quantum_properties) = compute_kind else {
Expand Down Expand Up @@ -307,6 +327,33 @@ impl<'a> Checker<'a> {
}
}

fn check_output_recording(&mut self, expr: &Expr) {
let compute_kind = self.compute_properties.get_expr(expr.id).inherent;
let ComputeKind::Quantum(quantum_properties) = compute_kind else {
return;
};

let output_reporting_span = match &expr.kind {
ExprKind::Call(callee_expr, _) if expr.span == Span::default() => {
// Since this is auto-generated, use the callee expression span.
self.get_expr(*callee_expr).span
}
_ => expr.span,
};

// Calculate the missing features but only consider the output recording flags.
let missing_features = get_missing_runtime_features(
quantum_properties.runtime_features,
self.target_capabilities,
) & RuntimeFeatureFlags::output_recording_flags();
if !missing_features.is_empty() {
self.missing_features_map
.entry(output_reporting_span)
.and_modify(|f| *f |= missing_features)
.or_insert(missing_features);
}
}

fn clear_current_callable(&mut self) -> LocalItemId {
self.current_callable
.take()
Expand Down
12 changes: 0 additions & 12 deletions compiler/qsc_passes/src/capabilitiesck/tests_adaptive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,12 +713,6 @@ fn use_of_static_int_in_tuple_return_from_entry_point_errors() {
USE_ENTRY_POINT_STATIC_INT_IN_TUPLE,
&expect![[r#"
[
UseOfDynamicInt(
Span {
lo: 63,
hi: 66,
},
),
UseOfIntOutput(
Span {
lo: 63,
Expand All @@ -736,12 +730,6 @@ fn use_of_static_sized_array_in_tuple_error() {
USE_ENTRY_POINT_INT_ARRAY_IN_TUPLE,
&expect![[r#"
[
UseOfDynamicInt(
Span {
lo: 63,
hi: 66,
},
),
UseOfIntOutput(
Span {
lo: 63,
Expand Down
12 changes: 0 additions & 12 deletions compiler/qsc_passes/src/capabilitiesck/tests_base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,12 +870,6 @@ fn use_of_static_int_in_tuple_return_from_entry_point_errors() {
USE_ENTRY_POINT_STATIC_INT_IN_TUPLE,
&expect![[r#"
[
UseOfDynamicInt(
Span {
lo: 63,
hi: 66,
},
),
UseOfIntOutput(
Span {
lo: 63,
Expand All @@ -893,12 +887,6 @@ fn use_of_static_sized_array_in_tuple_error() {
USE_ENTRY_POINT_INT_ARRAY_IN_TUPLE,
&expect![[r#"
[
UseOfDynamicInt(
Span {
lo: 63,
hi: 66,
},
),
UseOfIntOutput(
Span {
lo: 63,
Expand Down
6 changes: 3 additions & 3 deletions compiler/qsc_passes/src/entry_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ fn create_entry_from_callables(
qsc_hir::hir::SpecBody::Impl(_, block) => {
let arg = Expr {
id: assigner.next_node(),
span: ep.span,
span: ep.input.span,
ty: Ty::UNIT,
kind: ExprKind::Tuple(Vec::new()),
};
Expand All @@ -92,13 +92,13 @@ fn create_entry_from_callables(
};
let callee = Expr {
id: assigner.next_node(),
span: ep.span,
span: ep.name.span,
ty: block.ty.clone(),
kind: ExprKind::Var(Res::Item(item_id), Vec::new()),
};
let call = Expr {
id: assigner.next_node(),
span: ep.name.span,
span: Span::default(),
ty: block.ty.clone(),
kind: ExprKind::Call(Box::new(callee), Box::new(arg)),
};
Expand Down
6 changes: 3 additions & 3 deletions compiler/qsc_passes/src/entry_point/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ fn test_entry_point_attr_to_expr() {
}"},
"",
&expect![[r#"
Expr 12 [50-54] [Type Int]: Call:
Expr 11 [40-73] [Type Int]: Var: Item 1
Expr 10 [40-73] [Type Unit]: Unit"#]],
Expr 12 [0-0] [Type Int]: Call:
Expr 11 [50-54] [Type Int]: Var: Item 1
Expr 10 [54-56] [Type Unit]: Unit"#]],
);
}

Expand Down
11 changes: 8 additions & 3 deletions compiler/qsc_rca/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -846,9 +846,6 @@ impl RuntimeFeatureFlags {
if self.contains(RuntimeFeatureFlags::CallToDynamicCallee) {
capabilities |= TargetCapabilityFlags::HigherLevelConstructs;
}
if self.contains(RuntimeFeatureFlags::CallToUnresolvedCallee) {
capabilities |= TargetCapabilityFlags::HigherLevelConstructs;
}
if self.contains(RuntimeFeatureFlags::MeasurementWithinDynamicScope) {
capabilities |= TargetCapabilityFlags::Adaptive;
}
Expand Down Expand Up @@ -878,4 +875,12 @@ impl RuntimeFeatureFlags {
}
capabilities
}

#[must_use]
pub fn output_recording_flags() -> RuntimeFeatureFlags {
RuntimeFeatureFlags::UseOfIntOutput
| RuntimeFeatureFlags::UseOfDoubleOutput
| RuntimeFeatureFlags::UseOfBoolOutput
| RuntimeFeatureFlags::UseOfAdvancedOutput
}
}

0 comments on commit 5a60039

Please sign in to comment.