From b74867ceb788f490b95e6bb8a552faabc05c19ee Mon Sep 17 00:00:00 2001 From: "Stefan J. Wernli" Date: Thu, 9 Nov 2023 09:28:19 -0800 Subject: [PATCH] Use `Default` to handle missing entry in sequence (#836) This uses a more complex but ultimately more correct approach that fixes #672. By creating default error kinds for several of the AST and HIR types that can appear in sequences, sequence parsing recovery now inserts error entries to correspond to places where commas appear at the start of a sequence or back-to-back in the sequence. The end result is better signature help due to recovered HIR including placeholder error entries. --- compiler/qsc_ast/src/ast.rs | 112 +++++++++- compiler/qsc_ast/src/mut_visit.rs | 7 +- compiler/qsc_ast/src/visit.rs | 7 +- compiler/qsc_data_structures/src/span.rs | 6 + compiler/qsc_eval/src/lower.rs | 3 + compiler/qsc_frontend/src/lower.rs | 2 + compiler/qsc_frontend/src/lower/tests.rs | 159 ++++++++++++++ compiler/qsc_frontend/src/resolve.rs | 4 +- compiler/qsc_frontend/src/typeck/convert.rs | 10 +- compiler/qsc_frontend/src/typeck/rules.rs | 3 + compiler/qsc_hir/src/hir.rs | 6 + compiler/qsc_hir/src/mut_visit.rs | 4 +- compiler/qsc_hir/src/visit.rs | 4 +- compiler/qsc_parse/src/expr/tests.rs | 194 ++++++++++++++++++ compiler/qsc_parse/src/item.rs | 1 + compiler/qsc_parse/src/item/tests.rs | 110 ++++++++++ compiler/qsc_parse/src/lib.rs | 4 + compiler/qsc_parse/src/prim.rs | 21 +- compiler/qsc_parse/src/stmt/tests.rs | 87 ++++++++ compiler/qsc_passes/src/borrowck.rs | 2 +- .../src/replace_qubit_allocation.rs | 3 + language_service/src/display.rs | 4 + language_service/src/hover.rs | 2 +- language_service/src/signature_help.rs | 6 +- language_service/src/signature_help/tests.rs | 44 +++- 25 files changed, 773 insertions(+), 32 deletions(-) diff --git a/compiler/qsc_ast/src/ast.rs b/compiler/qsc_ast/src/ast.rs index 95a28f89f7..cca6b78e2b 100644 --- a/compiler/qsc_ast/src/ast.rs +++ b/compiler/qsc_ast/src/ast.rs @@ -7,7 +7,7 @@ use indenter::{indented, Format, Indented}; use num_bigint::BigInt; -use qsc_data_structures::span::Span; +use qsc_data_structures::span::{Span, WithSpan}; use std::{ cmp::Ordering, fmt::{self, Display, Formatter, Write}, @@ -107,6 +107,22 @@ impl Hash for NodeId { } } +/// Trait that allows creation of a default value with a span. +pub trait DefaultWithSpan { + /// Creates a default value with the given span by using the `Default` and `WithSpan` traits. + #[must_use] + fn default_with_span(span: Span) -> Self; +} + +impl DefaultWithSpan for Box +where + T: Default + WithSpan, +{ + fn default_with_span(span: Span) -> Self { + Box::new(T::default().with_span(span)) + } +} + /// The root node of an AST. #[derive(Clone, Debug, Default, PartialEq)] pub struct Package { @@ -317,7 +333,7 @@ impl Display for Attr { } /// A type definition. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Default)] pub struct TyDef { /// The node ID. pub id: NodeId, @@ -333,8 +349,14 @@ impl Display for TyDef { } } +impl WithSpan for TyDef { + fn with_span(self, span: Span) -> Self { + Self { span, ..self } + } +} + /// A type definition kind. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Default)] pub enum TyDefKind { /// A field definition with an optional name but required type. Field(Option>, Box), @@ -342,6 +364,9 @@ pub enum TyDefKind { Paren(Box), /// A tuple. Tuple(Box<[Box]>), + /// An invalid type definition. + #[default] + Err, } impl Display for TyDefKind { @@ -372,6 +397,7 @@ impl Display for TyDefKind { } } } + TyDefKind::Err => write!(indent, "Err")?, } Ok(()) } @@ -541,7 +567,7 @@ impl Display for FunctorExprKind { } /// A type. -#[derive(Clone, Debug, Eq, Hash, PartialEq)] +#[derive(Clone, Debug, Eq, Hash, PartialEq, Default)] pub struct Ty { /// The node ID. pub id: NodeId, @@ -557,8 +583,22 @@ impl Display for Ty { } } +impl WithSpan for Ty { + fn with_span(self, span: Span) -> Self { + Self { span, ..self } + } +} + +impl DefaultWithSpan for Ty { + /// Creates a default value with the given span by using the `Default` and `WithSpan` traits. + #[must_use] + fn default_with_span(span: Span) -> Self { + Self::default().with_span(span) + } +} + /// A type kind. -#[derive(Clone, Debug, Eq, Hash, PartialEq)] +#[derive(Clone, Debug, Eq, Hash, PartialEq, Default)] pub enum TyKind { /// An array type. Array(Box), @@ -574,6 +614,9 @@ pub enum TyKind { Param(Box), /// A tuple type. Tuple(Box<[Ty]>), + /// An invalid type. + #[default] + Err, } impl Display for TyKind { @@ -607,6 +650,7 @@ impl Display for TyKind { } } } + TyKind::Err => write!(indent, "Err")?, } Ok(()) } @@ -722,6 +766,12 @@ impl Display for Expr { } } +impl WithSpan for Expr { + fn with_span(self, span: Span) -> Self { + Self { span, ..self } + } +} + /// An expression kind. #[derive(Clone, Debug, Default, PartialEq)] pub enum ExprKind { @@ -1082,7 +1132,7 @@ pub enum StringComponent { } /// A pattern. -#[derive(Clone, Debug, Eq, Hash, PartialEq)] +#[derive(Clone, Debug, Eq, Hash, PartialEq, Default)] pub struct Pat { /// The node ID. pub id: NodeId, @@ -1098,8 +1148,14 @@ impl Display for Pat { } } +impl WithSpan for Pat { + fn with_span(self, span: Span) -> Self { + Self { span, ..self } + } +} + /// A pattern kind. -#[derive(Clone, Debug, Eq, Hash, PartialEq)] +#[derive(Clone, Debug, Eq, Hash, PartialEq, Default)] pub enum PatKind { /// A binding with an optional type annotation. Bind(Box, Option>), @@ -1111,6 +1167,9 @@ pub enum PatKind { Paren(Box), /// A tuple: `(a, b, c)`. Tuple(Box<[Box]>), + /// An invalid pattern. + #[default] + Err, } impl Display for PatKind { @@ -1150,13 +1209,14 @@ impl Display for PatKind { } } } + PatKind::Err => write!(indent, "Err")?, } Ok(()) } } /// A qubit initializer. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Default)] pub struct QubitInit { /// The node ID. pub id: NodeId, @@ -1172,8 +1232,14 @@ impl Display for QubitInit { } } +impl WithSpan for QubitInit { + fn with_span(self, span: Span) -> Self { + Self { span, ..self } + } +} + /// A qubit initializer kind. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq, Default)] pub enum QubitInitKind { /// An array of qubits: `Qubit[a]`. Array(Box), @@ -1183,6 +1249,9 @@ pub enum QubitInitKind { Single, /// A tuple: `(a, b, c)`. Tuple(Box<[Box]>), + /// An invalid initializer. + #[default] + Err, } impl Display for QubitInitKind { @@ -1211,13 +1280,14 @@ impl Display for QubitInitKind { } } } + QubitInitKind::Err => write!(indent, "Err")?, } Ok(()) } } /// A path to a declaration. -#[derive(Clone, Debug, Eq, Hash, PartialEq)] +#[derive(Clone, Debug, Eq, Hash, PartialEq, Default)] pub struct Path { /// The node ID. pub id: NodeId, @@ -1240,6 +1310,12 @@ impl Display for Path { } } +impl WithSpan for Path { + fn with_span(self, span: Span) -> Self { + Self { span, ..self } + } +} + /// An identifier. #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct Ident { @@ -1251,6 +1327,22 @@ pub struct Ident { pub name: Rc, } +impl Default for Ident { + fn default() -> Self { + Ident { + id: NodeId::default(), + span: Span::default(), + name: "".into(), + } + } +} + +impl WithSpan for Ident { + fn with_span(self, span: Span) -> Self { + Self { span, ..self } + } +} + impl Display for Ident { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!(f, "Ident {} {} \"{}\"", self.id, self.span, self.name) diff --git a/compiler/qsc_ast/src/mut_visit.rs b/compiler/qsc_ast/src/mut_visit.rs index b1a445aea1..8b47a31bb4 100644 --- a/compiler/qsc_ast/src/mut_visit.rs +++ b/compiler/qsc_ast/src/mut_visit.rs @@ -130,6 +130,7 @@ pub fn walk_ty_def(vis: &mut impl MutVisitor, def: &mut TyDef) { } TyDefKind::Paren(def) => vis.visit_ty_def(def), TyDefKind::Tuple(defs) => defs.iter_mut().for_each(|d| vis.visit_ty_def(d)), + TyDefKind::Err => {} } } @@ -184,7 +185,7 @@ pub fn walk_ty(vis: &mut impl MutVisitor, ty: &mut Ty) { vis.visit_ty(rhs); functors.iter_mut().for_each(|f| vis.visit_functor_expr(f)); } - TyKind::Hole => {} + TyKind::Hole | TyKind::Err => {} TyKind::Paren(ty) => vis.visit_ty(ty), TyKind::Param(name) => vis.visit_ident(name), TyKind::Path(path) => vis.visit_path(path), @@ -313,7 +314,7 @@ pub fn walk_pat(vis: &mut impl MutVisitor, pat: &mut Pat) { ty.iter_mut().for_each(|t| vis.visit_ty(t)); } PatKind::Discard(ty) => ty.iter_mut().for_each(|t| vis.visit_ty(t)), - PatKind::Elided => {} + PatKind::Elided | PatKind::Err => {} PatKind::Paren(pat) => vis.visit_pat(pat), PatKind::Tuple(pats) => pats.iter_mut().for_each(|p| vis.visit_pat(p)), } @@ -325,7 +326,7 @@ pub fn walk_qubit_init(vis: &mut impl MutVisitor, init: &mut QubitInit) { match &mut *init.kind { QubitInitKind::Array(len) => vis.visit_expr(len), QubitInitKind::Paren(init) => vis.visit_qubit_init(init), - QubitInitKind::Single => {} + QubitInitKind::Single | QubitInitKind::Err => {} QubitInitKind::Tuple(inits) => inits.iter_mut().for_each(|i| vis.visit_qubit_init(i)), } } diff --git a/compiler/qsc_ast/src/visit.rs b/compiler/qsc_ast/src/visit.rs index d80b09cacc..f9afe8b098 100644 --- a/compiler/qsc_ast/src/visit.rs +++ b/compiler/qsc_ast/src/visit.rs @@ -117,6 +117,7 @@ pub fn walk_ty_def<'a>(vis: &mut impl Visitor<'a>, def: &'a TyDef) { } TyDefKind::Paren(def) => vis.visit_ty_def(def), TyDefKind::Tuple(defs) => defs.iter().for_each(|d| vis.visit_ty_def(d)), + TyDefKind::Err => {} } } @@ -161,7 +162,7 @@ pub fn walk_ty<'a>(vis: &mut impl Visitor<'a>, ty: &'a Ty) { vis.visit_ty(rhs); functors.iter().for_each(|f| vis.visit_functor_expr(f)); } - TyKind::Hole => {} + TyKind::Hole | TyKind::Err => {} TyKind::Paren(ty) => vis.visit_ty(ty), TyKind::Path(path) => vis.visit_path(path), TyKind::Param(name) => vis.visit_ident(name), @@ -283,7 +284,7 @@ pub fn walk_pat<'a>(vis: &mut impl Visitor<'a>, pat: &'a Pat) { ty.iter().for_each(|t| vis.visit_ty(t)); } PatKind::Discard(ty) => ty.iter().for_each(|t| vis.visit_ty(t)), - PatKind::Elided => {} + PatKind::Elided | PatKind::Err => {} PatKind::Paren(pat) => vis.visit_pat(pat), PatKind::Tuple(pats) => pats.iter().for_each(|p| vis.visit_pat(p)), } @@ -293,7 +294,7 @@ pub fn walk_qubit_init<'a>(vis: &mut impl Visitor<'a>, init: &'a QubitInit) { match &*init.kind { QubitInitKind::Array(len) => vis.visit_expr(len), QubitInitKind::Paren(init) => vis.visit_qubit_init(init), - QubitInitKind::Single => {} + QubitInitKind::Single | QubitInitKind::Err => {} QubitInitKind::Tuple(inits) => inits.iter().for_each(|i| vis.visit_qubit_init(i)), } } diff --git a/compiler/qsc_data_structures/src/span.rs b/compiler/qsc_data_structures/src/span.rs index 9535d33204..0823d42bb5 100644 --- a/compiler/qsc_data_structures/src/span.rs +++ b/compiler/qsc_data_structures/src/span.rs @@ -74,3 +74,9 @@ impl From for SourceSpan { Self::from((value.lo as usize)..(value.hi as usize)) } } + +#[allow(clippy::module_name_repetitions)] +pub trait WithSpan { + #[must_use] + fn with_span(self, span: Span) -> Self; +} diff --git a/compiler/qsc_eval/src/lower.rs b/compiler/qsc_eval/src/lower.rs index d15e906d9f..e886b3a942 100644 --- a/compiler/qsc_eval/src/lower.rs +++ b/compiler/qsc_eval/src/lower.rs @@ -213,6 +213,7 @@ impl Lowerer { hir::PatKind::Tuple(elems) => { fir::PatKind::Tuple(elems.iter().map(|pat| self.lower_pat(pat)).collect()) } + hir::PatKind::Err => unreachable!("error pat should not be present"), }; let pat = fir::Pat { id, span, ty, kind }; @@ -389,6 +390,7 @@ impl Lowerer { hir::PatKind::Tuple(items) => { fir::PatKind::Tuple(items.iter().map(|i| self.lower_pat(i)).collect()) } + hir::PatKind::Err => unreachable!("error pat should not be present"), }; let pat = fir::Pat { @@ -410,6 +412,7 @@ impl Lowerer { hir::QubitInitKind::Tuple(items) => { fir::QubitInitKind::Tuple(items.iter().map(|i| self.lower_qubit_init(i)).collect()) } + hir::QubitInitKind::Err => unreachable!("error qubit init should not be present"), }; fir::QubitInit { diff --git a/compiler/qsc_frontend/src/lower.rs b/compiler/qsc_frontend/src/lower.rs index bdb312d2fe..9664278f1d 100644 --- a/compiler/qsc_frontend/src/lower.rs +++ b/compiler/qsc_frontend/src/lower.rs @@ -666,6 +666,7 @@ impl With<'_> { ast::PatKind::Tuple(items) => { hir::PatKind::Tuple(items.iter().map(|i| self.lower_pat(i)).collect()) } + ast::PatKind::Err => hir::PatKind::Err, }; hir::Pat { @@ -692,6 +693,7 @@ impl With<'_> { ast::QubitInitKind::Tuple(items) => { hir::QubitInitKind::Tuple(items.iter().map(|i| self.lower_qubit_init(i)).collect()) } + ast::QubitInitKind::Err => hir::QubitInitKind::Err, }; hir::QubitInit { diff --git a/compiler/qsc_frontend/src/lower/tests.rs b/compiler/qsc_frontend/src/lower/tests.rs index 930fdd66eb..91eb19df67 100644 --- a/compiler/qsc_frontend/src/lower/tests.rs +++ b/compiler/qsc_frontend/src/lower/tests.rs @@ -2030,3 +2030,162 @@ fn lambda_with_invalid_free_variable() { ctl-adj: "#]], ); } + +#[test] +fn duplicate_commas_in_tydef() { + check_hir( + indoc! {r#" + namespace test { + newtype Foo = (Int,,); + } + "#}, + &expect![[r#" + Package: + Item 0 [0-45] (Public): + Namespace (Ident 1 [10-14] "test"): Item 1 + Item 1 [21-43] (Public): + Parent: 0 + Type (Ident 0 [29-32] "Foo"): UDT [21-43]: + TyDef [35-42]: Tuple: + TyDef [36-39]: Field: + type: Int + TyDef [40-40]: Field: + type: ?"#]], + ); +} + +#[test] +fn duplicate_commas_in_generics() { + check_hir( + indoc! {r#" + namespace test { + function Foo<'T,,>(x : 'T) : Unit {} + } + "#}, + &expect![[r#" + Package: + Item 0 [0-59] (Public): + Namespace (Ident 6 [10-14] "test"): Item 1 + Item 1 [21-57] (Public): + Parent: 0 + Callable 0 [21-57] (function): + name: Ident 1 [30-33] "Foo" + generics: + 0: type + 1: type + input: Pat 2 [40-46] [Type 0]: Bind: Ident 3 [40-41] "x" + output: Unit + functors: empty set + body: SpecDecl 4 [21-57]: Impl: + Block 5 [55-57]: + adj: + ctl: + ctl-adj: "#]], + ); +} + +#[test] +fn duplicate_commas_in_pat() { + check_hir( + indoc! {r#" + namespace test { + operation Foo() : Unit { + let (x,,) = (1, 2); + } + }"#}, + &expect![[r#" + Package: + Item 0 [0-81] (Public): + Namespace (Ident 13 [10-14] "test"): Item 1 + Item 1 [21-79] (Public): + Parent: 0 + Callable 0 [21-79] (operation): + name: Ident 1 [31-34] "Foo" + input: Pat 2 [34-36] [Type Unit]: Unit + output: Unit + functors: empty set + body: SpecDecl 3 [21-79]: Impl: + Block 4 [44-79] [Type Unit]: + Stmt 5 [54-73]: Local (Immutable): + Pat 6 [58-63] [Type (Int, ?)]: Tuple: + Pat 7 [59-60] [Type Int]: Bind: Ident 8 [59-60] "x" + Pat 9 [61-61] [Type ?]: Err + Expr 10 [66-72] [Type (Int, Int)]: Tuple: + Expr 11 [67-68] [Type Int]: Lit: Int(1) + Expr 12 [70-71] [Type Int]: Lit: Int(2) + adj: + ctl: + ctl-adj: "#]], + ); +} + +#[test] +fn duplicate_commas_in_tuple() { + check_hir( + indoc! {r#" + namespace test { + operation Foo() : Unit { + let x = (1,,3); + } + }"#}, + &expect![[r#" + Package: + Item 0 [0-77] (Public): + Namespace (Ident 12 [10-14] "test"): Item 1 + Item 1 [21-75] (Public): + Parent: 0 + Callable 0 [21-75] (operation): + name: Ident 1 [31-34] "Foo" + input: Pat 2 [34-36] [Type Unit]: Unit + output: Unit + functors: empty set + body: SpecDecl 3 [21-75]: Impl: + Block 4 [44-75] [Type Unit]: + Stmt 5 [54-69]: Local (Immutable): + Pat 6 [58-59] [Type (Int, ?, Int)]: Bind: Ident 7 [58-59] "x" + Expr 8 [62-68] [Type (Int, ?, Int)]: Tuple: + Expr 9 [63-64] [Type Int]: Lit: Int(1) + Expr 10 [65-65] [Type ?]: Err + Expr 11 [66-67] [Type Int]: Lit: Int(3) + adj: + ctl: + ctl-adj: "#]], + ); +} + +#[test] +fn duplicate_commas_in_arg_tuple() { + check_hir( + indoc! {r#" + namespace test { + operation Foo(a : Int, b : Int, c : Int) : Int { + Foo(a,,c) + } + }"#}, + &expect![[r#" + Package: + Item 0 [0-95] (Public): + Namespace (Ident 18 [10-14] "test"): Item 1 + Item 1 [21-93] (Public): + Parent: 0 + Callable 0 [21-93] (operation): + name: Ident 1 [31-34] "Foo" + input: Pat 2 [34-61] [Type (Int, Int, Int)]: Tuple: + Pat 3 [35-42] [Type Int]: Bind: Ident 4 [35-36] "a" + Pat 5 [44-51] [Type Int]: Bind: Ident 6 [44-45] "b" + Pat 7 [53-60] [Type Int]: Bind: Ident 8 [53-54] "c" + output: Int + functors: empty set + body: SpecDecl 9 [21-93]: Impl: + Block 10 [68-93] [Type Int]: + Stmt 11 [78-87]: Expr: Expr 12 [78-87] [Type Int]: Call: + Expr 13 [78-81] [Type ((Int, Int, Int) => Int)]: Var: Item 1 + Expr 14 [81-87] [Type (Int, ?, Int)]: Tuple: + Expr 15 [82-83] [Type Int]: Var: Local 4 + Expr 16 [84-84] [Type ?]: Err + Expr 17 [85-86] [Type Int]: Var: Local 8 + adj: + ctl: + ctl-adj: "#]], + ); +} diff --git a/compiler/qsc_frontend/src/resolve.rs b/compiler/qsc_frontend/src/resolve.rs index d138cce89c..a2ef7b81cd 100644 --- a/compiler/qsc_frontend/src/resolve.rs +++ b/compiler/qsc_frontend/src/resolve.rs @@ -287,7 +287,7 @@ impl Resolver { self.names.insert(name.id, Res::Local(name.id)); scope.vars.insert(Rc::clone(&name.name), name.id); } - ast::PatKind::Discard(_) | ast::PatKind::Elided => {} + ast::PatKind::Discard(_) | ast::PatKind::Elided | ast::PatKind::Err => {} ast::PatKind::Paren(pat) => self.bind_pat_recursive(pat, bindings), ast::PatKind::Tuple(pats) => pats .iter() @@ -399,7 +399,7 @@ impl AstVisitor<'_> for With<'_> { ast::PatKind::Bind(name, _) => { names.insert(Rc::clone(&name.name)); } - ast::PatKind::Discard(_) | ast::PatKind::Elided => {} + ast::PatKind::Discard(_) | ast::PatKind::Elided | ast::PatKind::Err => {} ast::PatKind::Paren(pat) => collect_param_names(pat, names), ast::PatKind::Tuple(pats) => { pats.iter().for_each(|p| collect_param_names(p, names)); diff --git a/compiler/qsc_frontend/src/typeck/convert.rs b/compiler/qsc_frontend/src/typeck/convert.rs index 172beb6bb7..54fcad7cbd 100644 --- a/compiler/qsc_frontend/src/typeck/convert.rs +++ b/compiler/qsc_frontend/src/typeck/convert.rs @@ -75,6 +75,7 @@ pub(crate) fn ty_from_ast(names: &Names, ty: &ast::Ty) -> (Ty, Vec (Ty::Err, Vec::new()), } } @@ -109,6 +110,7 @@ fn ast_ty_def_base(names: &Names, def: &TyDef) -> (Ty, Vec) { (Ty::Tuple(tys), errors) } + TyDefKind::Err => (Ty::Err, Vec::new()), } } @@ -146,6 +148,11 @@ pub(super) fn ast_ty_def(names: &Names, def: &TyDef) -> (UdtDef, Vec UdtDefKind::Field(UdtField { + name_span: None, + name: None, + ty: Ty::Err, + }), }, }; @@ -210,7 +217,7 @@ fn synthesize_functor_params_in_pat( pat: &mut hir::Pat, ) -> Vec { match &mut pat.kind { - hir::PatKind::Discard | hir::PatKind::Bind(_) => { + hir::PatKind::Discard | hir::PatKind::Err | hir::PatKind::Bind(_) => { synthesize_functor_params(next_param, &mut pat.ty) } hir::PatKind::Tuple(items) => { @@ -247,6 +254,7 @@ pub(crate) fn ast_pat_ty(names: &Names, pat: &Pat) -> (Ty, Vec) } (Ty::Tuple(tys), errors) } + PatKind::Err => (Ty::Err, Vec::new()), } } diff --git a/compiler/qsc_frontend/src/typeck/rules.rs b/compiler/qsc_frontend/src/typeck/rules.rs index b100ca5265..b3b8750bab 100644 --- a/compiler/qsc_frontend/src/typeck/rules.rs +++ b/compiler/qsc_frontend/src/typeck/rules.rs @@ -126,6 +126,7 @@ impl<'a> Context<'a> { TyKind::Tuple(items) => { Ty::Tuple(items.iter().map(|item| self.infer_ty(item)).collect()) } + TyKind::Err => Ty::Err, } } @@ -666,6 +667,7 @@ impl<'a> Context<'a> { PatKind::Tuple(items) => { Ty::Tuple(items.iter().map(|item| self.infer_pat(item)).collect()) } + PatKind::Err => Ty::Err, }; self.record(pat.id, ty.clone()); @@ -696,6 +698,7 @@ impl<'a> Context<'a> { } self.diverge_if(diverges, converge(Ty::Tuple(tys))) } + QubitInitKind::Err => converge(Ty::Err), }; self.record(init.id, ty.ty.clone()); diff --git a/compiler/qsc_hir/src/hir.rs b/compiler/qsc_hir/src/hir.rs index e51929234f..dc0c1a5dc1 100644 --- a/compiler/qsc_hir/src/hir.rs +++ b/compiler/qsc_hir/src/hir.rs @@ -1015,6 +1015,8 @@ pub enum PatKind { Discard, /// A tuple: `(a, b, c)`. Tuple(Vec), + /// An invalid pattern. + Err, } impl Display for PatKind { @@ -1036,6 +1038,7 @@ impl Display for PatKind { } } } + PatKind::Err => write!(indent, "Err")?, } Ok(()) } @@ -1073,6 +1076,8 @@ pub enum QubitInitKind { Single, /// A tuple: `(a, b, c)`. Tuple(Vec), + /// An invalid qubit initializer. + Err, } impl Display for QubitInitKind { @@ -1096,6 +1101,7 @@ impl Display for QubitInitKind { } } } + QubitInitKind::Err => write!(indent, "Err")?, } Ok(()) } diff --git a/compiler/qsc_hir/src/mut_visit.rs b/compiler/qsc_hir/src/mut_visit.rs index 484214103e..00333b70d3 100644 --- a/compiler/qsc_hir/src/mut_visit.rs +++ b/compiler/qsc_hir/src/mut_visit.rs @@ -210,7 +210,7 @@ pub fn walk_pat(vis: &mut impl MutVisitor, pat: &mut Pat) { match &mut pat.kind { PatKind::Bind(name) => vis.visit_ident(name), - PatKind::Discard => {} + PatKind::Discard | PatKind::Err => {} PatKind::Tuple(pats) => pats.iter_mut().for_each(|p| vis.visit_pat(p)), } } @@ -220,7 +220,7 @@ pub fn walk_qubit_init(vis: &mut impl MutVisitor, init: &mut QubitInit) { match &mut init.kind { QubitInitKind::Array(len) => vis.visit_expr(len), - QubitInitKind::Single => {} + QubitInitKind::Single | QubitInitKind::Err => {} QubitInitKind::Tuple(inits) => inits.iter_mut().for_each(|i| vis.visit_qubit_init(i)), } } diff --git a/compiler/qsc_hir/src/visit.rs b/compiler/qsc_hir/src/visit.rs index f979e38130..6f9655c29a 100644 --- a/compiler/qsc_hir/src/visit.rs +++ b/compiler/qsc_hir/src/visit.rs @@ -189,7 +189,7 @@ pub fn walk_expr<'a>(vis: &mut impl Visitor<'a>, expr: &'a Expr) { pub fn walk_pat<'a>(vis: &mut impl Visitor<'a>, pat: &'a Pat) { match &pat.kind { PatKind::Bind(name) => vis.visit_ident(name), - PatKind::Discard => {} + PatKind::Discard | PatKind::Err => {} PatKind::Tuple(pats) => pats.iter().for_each(|p| vis.visit_pat(p)), } } @@ -197,7 +197,7 @@ pub fn walk_pat<'a>(vis: &mut impl Visitor<'a>, pat: &'a Pat) { pub fn walk_qubit_init<'a>(vis: &mut impl Visitor<'a>, init: &'a QubitInit) { match &init.kind { QubitInitKind::Array(len) => vis.visit_expr(len), - QubitInitKind::Single => {} + QubitInitKind::Single | QubitInitKind::Err => {} QubitInitKind::Tuple(inits) => inits.iter().for_each(|i| vis.visit_qubit_init(i)), } } diff --git a/compiler/qsc_parse/src/expr/tests.rs b/compiler/qsc_parse/src/expr/tests.rs index 0dc826b72b..68d1fa5e0c 100644 --- a/compiler/qsc_parse/src/expr/tests.rs +++ b/compiler/qsc_parse/src/expr/tests.rs @@ -2239,3 +2239,197 @@ fn nested_interpolated_string_with_exprs() { Lit: " baz""#]], ); } + +#[test] +fn duplicate_commas_in_tuple() { + check( + expr, + "(x,, y)", + &expect![[r#" + Expr _id_ [0-7]: Tuple: + Expr _id_ [1-2]: Path: Path _id_ [1-2] (Ident _id_ [1-2] "x") + Expr _id_ [3-3]: Err + Expr _id_ [5-6]: Path: Path _id_ [5-6] (Ident _id_ [5-6] "y") + + [ + Error( + MissingSeqEntry( + Span { + lo: 3, + hi: 3, + }, + ), + ), + ]"#]], + ); +} + +#[test] +fn many_duplicate_commas_in_tuple() { + check( + expr, + "(x,,,, y)", + &expect![[r#" + Expr _id_ [0-9]: Tuple: + Expr _id_ [1-2]: Path: Path _id_ [1-2] (Ident _id_ [1-2] "x") + Expr _id_ [3-3]: Err + Expr _id_ [4-4]: Err + Expr _id_ [5-5]: Err + Expr _id_ [7-8]: Path: Path _id_ [7-8] (Ident _id_ [7-8] "y") + + [ + Error( + MissingSeqEntry( + Span { + lo: 3, + hi: 3, + }, + ), + ), + Error( + MissingSeqEntry( + Span { + lo: 4, + hi: 4, + }, + ), + ), + Error( + MissingSeqEntry( + Span { + lo: 5, + hi: 5, + }, + ), + ), + ]"#]], + ); +} + +#[test] +fn invalid_initial_comma_in_tuple() { + check( + expr, + "(, x)", + &expect![[r#" + Expr _id_ [0-5]: Tuple: + Expr _id_ [1-1]: Err + Expr _id_ [3-4]: Path: Path _id_ [3-4] (Ident _id_ [3-4] "x") + + [ + Error( + MissingSeqEntry( + Span { + lo: 1, + hi: 1, + }, + ), + ), + ]"#]], + ); +} + +#[test] +fn many_invalid_initial_commas_in_tuple() { + check( + expr, + "(,,,, x)", + &expect![[r#" + Expr _id_ [0-8]: Tuple: + Expr _id_ [1-1]: Err + Expr _id_ [2-2]: Err + Expr _id_ [3-3]: Err + Expr _id_ [4-4]: Err + Expr _id_ [6-7]: Path: Path _id_ [6-7] (Ident _id_ [6-7] "x") + + [ + Error( + MissingSeqEntry( + Span { + lo: 1, + hi: 1, + }, + ), + ), + Error( + MissingSeqEntry( + Span { + lo: 2, + hi: 2, + }, + ), + ), + Error( + MissingSeqEntry( + Span { + lo: 3, + hi: 3, + }, + ), + ), + Error( + MissingSeqEntry( + Span { + lo: 4, + hi: 4, + }, + ), + ), + ]"#]], + ); +} + +#[test] +fn duplicate_commas_in_pattern() { + check( + expr, + "set (x,, y) = (1, 2)", + &expect![[r#" + Expr _id_ [0-20]: Assign: + Expr _id_ [4-11]: Tuple: + Expr _id_ [5-6]: Path: Path _id_ [5-6] (Ident _id_ [5-6] "x") + Expr _id_ [7-7]: Err + Expr _id_ [9-10]: Path: Path _id_ [9-10] (Ident _id_ [9-10] "y") + Expr _id_ [14-20]: Tuple: + Expr _id_ [15-16]: Lit: Int(1) + Expr _id_ [18-19]: Lit: Int(2) + + [ + Error( + MissingSeqEntry( + Span { + lo: 7, + hi: 7, + }, + ), + ), + ]"#]], + ); +} + +#[test] +fn invalid_initial_commas_in_pattern() { + check( + expr, + "set (, x) = (1, 2)", + &expect![[r#" + Expr _id_ [0-18]: Assign: + Expr _id_ [4-9]: Tuple: + Expr _id_ [5-5]: Err + Expr _id_ [7-8]: Path: Path _id_ [7-8] (Ident _id_ [7-8] "x") + Expr _id_ [12-18]: Tuple: + Expr _id_ [13-14]: Lit: Int(1) + Expr _id_ [16-17]: Lit: Int(2) + + [ + Error( + MissingSeqEntry( + Span { + lo: 5, + hi: 5, + }, + ), + ), + ]"#]], + ); +} diff --git a/compiler/qsc_parse/src/item.rs b/compiler/qsc_parse/src/item.rs index 552d854ca6..412b788dde 100644 --- a/compiler/qsc_parse/src/item.rs +++ b/compiler/qsc_parse/src/item.rs @@ -230,6 +230,7 @@ fn try_tydef_as_ty(tydef: &TyDef) -> Option { kind: Box::new(TyKind::Tuple(ty_tup.into_boxed_slice())), }) } + TyDefKind::Err => None, } } diff --git a/compiler/qsc_parse/src/item/tests.rs b/compiler/qsc_parse/src/item/tests.rs index ceb877c25f..a83a4692f7 100644 --- a/compiler/qsc_parse/src/item/tests.rs +++ b/compiler/qsc_parse/src/item/tests.rs @@ -300,6 +300,86 @@ fn ty_def_tuple_lambda_args() { ); } +#[test] +fn ty_def_duplicate_comma() { + check( + parse, + "newtype Foo = (Int,, Int);", + &expect![[r#" + Item _id_ [0-26]: + New Type (Ident _id_ [8-11] "Foo"): TyDef _id_ [14-25]: Tuple: + TyDef _id_ [15-18]: Field: + Type _id_ [15-18]: Path: Path _id_ [15-18] (Ident _id_ [15-18] "Int") + TyDef _id_ [19-19]: Err + TyDef _id_ [21-24]: Field: + Type _id_ [21-24]: Path: Path _id_ [21-24] (Ident _id_ [21-24] "Int") + + [ + Error( + MissingSeqEntry( + Span { + lo: 19, + hi: 19, + }, + ), + ), + ]"#]], + ); +} + +#[test] +fn ty_def_initial_comma() { + check( + parse, + "newtype Foo = (, Int);", + &expect![[r#" + Item _id_ [0-22]: + New Type (Ident _id_ [8-11] "Foo"): TyDef _id_ [14-21]: Tuple: + TyDef _id_ [15-15]: Err + TyDef _id_ [17-20]: Field: + Type _id_ [17-20]: Path: Path _id_ [17-20] (Ident _id_ [17-20] "Int") + + [ + Error( + MissingSeqEntry( + Span { + lo: 15, + hi: 15, + }, + ), + ), + ]"#]], + ); +} + +#[test] +fn ty_def_named_duplicate_comma() { + check( + parse, + "newtype Foo = (X : Int,, Int);", + &expect![[r#" + Item _id_ [0-30]: + New Type (Ident _id_ [8-11] "Foo"): TyDef _id_ [14-29]: Tuple: + TyDef _id_ [15-22]: Field: + Ident _id_ [15-16] "X" + Type _id_ [19-22]: Path: Path _id_ [19-22] (Ident _id_ [19-22] "Int") + TyDef _id_ [23-23]: Err + TyDef _id_ [25-28]: Field: + Type _id_ [25-28]: Path: Path _id_ [25-28] (Ident _id_ [25-28] "Int") + + [ + Error( + MissingSeqEntry( + Span { + lo: 23, + hi: 23, + }, + ), + ), + ]"#]], + ); +} + #[test] fn function_decl() { check( @@ -450,6 +530,36 @@ fn function_two_ty_params() { ); } +#[test] +fn function_duplicate_comma_in_ty_param() { + check( + parse, + "function Foo<'T,,>() : Unit { body intrinsic; }", + &expect![[r#" + Item _id_ [0-47]: + Callable _id_ [0-47] (Function): + name: Ident _id_ [9-12] "Foo" + generics: + Ident _id_ [13-15] "'T" + Ident _id_ [16-16] "" + input: Pat _id_ [18-20]: Unit + output: Type _id_ [23-27]: Path: Path _id_ [23-27] (Ident _id_ [23-27] "Unit") + body: Specializations: + SpecDecl _id_ [30-45] (Body): Gen: Intrinsic + + [ + Error( + MissingSeqEntry( + Span { + lo: 16, + hi: 16, + }, + ), + ), + ]"#]], + ); +} + #[test] fn function_single_impl() { check( diff --git a/compiler/qsc_parse/src/lib.rs b/compiler/qsc_parse/src/lib.rs index 839ef2cce4..834c43b065 100644 --- a/compiler/qsc_parse/src/lib.rs +++ b/compiler/qsc_parse/src/lib.rs @@ -70,6 +70,9 @@ enum ErrorKind { #[error("expected callable inputs to be parenthesized")] #[diagnostic(code("Qsc.Parse.MissingParens"))] MissingParens(#[label] Span), + #[error("missing entry in sequence")] + #[diagnostic(code("Qsc.Parse.MissingSeqEntry"))] + MissingSeqEntry(#[label] Span), } impl ErrorKind { @@ -86,6 +89,7 @@ impl ErrorKind { Self::FloatingDocComment(span) => Self::FloatingDocComment(span + offset), Self::FloatingAttr(span) => Self::FloatingAttr(span + offset), Self::FloatingVisibility(span) => Self::FloatingVisibility(span + offset), + Self::MissingSeqEntry(span) => Self::MissingSeqEntry(span + offset), } } } diff --git a/compiler/qsc_parse/src/prim.rs b/compiler/qsc_parse/src/prim.rs index e46a7d631f..d837453012 100644 --- a/compiler/qsc_parse/src/prim.rs +++ b/compiler/qsc_parse/src/prim.rs @@ -9,7 +9,7 @@ use crate::{ lex::{Delim, TokenKind}, ErrorKind, }; -use qsc_ast::ast::{Ident, NodeId, Pat, PatKind, Path}; +use qsc_ast::ast::{DefaultWithSpan, Ident, NodeId, Pat, PatKind, Path}; use qsc_data_structures::span::Span; #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -170,12 +170,29 @@ pub(super) fn many(s: &mut Scanner, mut p: impl Parser) -> Result> Ok(xs) } -pub(super) fn seq(s: &mut Scanner, mut p: impl Parser) -> Result<(Vec, FinalSep)> { +pub(super) fn seq(s: &mut Scanner, mut p: impl Parser) -> Result<(Vec, FinalSep)> +where + T: DefaultWithSpan, +{ let mut xs = Vec::new(); let mut final_sep = FinalSep::Missing; + while s.peek().kind == TokenKind::Comma { + let mut span = s.peek().span; + span.hi = span.lo; + s.push_error(Error(ErrorKind::MissingSeqEntry(span))); + xs.push(T::default_with_span(span)); + s.advance(); + } while let Some(x) = opt(s, &mut p)? { xs.push(x); if token(s, TokenKind::Comma).is_ok() { + while s.peek().kind == TokenKind::Comma { + let mut span = s.peek().span; + span.hi = span.lo; + s.push_error(Error(ErrorKind::MissingSeqEntry(span))); + xs.push(T::default_with_span(span)); + s.advance(); + } final_sep = FinalSep::Present; } else { final_sep = FinalSep::Missing; diff --git a/compiler/qsc_parse/src/stmt/tests.rs b/compiler/qsc_parse/src/stmt/tests.rs index bee9f08b1b..a7d8a5127e 100644 --- a/compiler/qsc_parse/src/stmt/tests.rs +++ b/compiler/qsc_parse/src/stmt/tests.rs @@ -149,6 +149,93 @@ fn use_invalid_init() { ); } +#[test] +fn use_tuple_duplicate_commas() { + check( + parse, + "use (q1,, q2) = (Qubit(),, Qubit());", + &expect![[r#" + Stmt _id_ [0-36]: Qubit (Fresh) + Pat _id_ [4-13]: Tuple: + Pat _id_ [5-7]: Bind: + Ident _id_ [5-7] "q1" + Pat _id_ [8-8]: Err + Pat _id_ [10-12]: Bind: + Ident _id_ [10-12] "q2" + QubitInit _id_ [16-35] Tuple: + QubitInit _id_ [17-24] Single + QubitInit _id_ [25-25] Err + QubitInit _id_ [27-34] Single + + [ + Error( + MissingSeqEntry( + Span { + lo: 8, + hi: 8, + }, + ), + ), + Error( + MissingSeqEntry( + Span { + lo: 25, + hi: 25, + }, + ), + ), + ]"#]], + ); +} + +#[test] +fn use_tuple_initial_commas() { + check( + parse, + "use (,, q1, q2) = (, Qubit(), Qubit());", + &expect![[r#" + Stmt _id_ [0-39]: Qubit (Fresh) + Pat _id_ [4-15]: Tuple: + Pat _id_ [5-5]: Err + Pat _id_ [6-6]: Err + Pat _id_ [8-10]: Bind: + Ident _id_ [8-10] "q1" + Pat _id_ [12-14]: Bind: + Ident _id_ [12-14] "q2" + QubitInit _id_ [18-38] Tuple: + QubitInit _id_ [19-19] Err + QubitInit _id_ [21-28] Single + QubitInit _id_ [30-37] Single + + [ + Error( + MissingSeqEntry( + Span { + lo: 5, + hi: 5, + }, + ), + ), + Error( + MissingSeqEntry( + Span { + lo: 6, + hi: 6, + }, + ), + ), + Error( + MissingSeqEntry( + Span { + lo: 19, + hi: 19, + }, + ), + ), + ]"#]], + ); +} + #[test] fn borrow_stmt() { check( diff --git a/compiler/qsc_passes/src/borrowck.rs b/compiler/qsc_passes/src/borrowck.rs index 3fc1938740..84181fb4d5 100644 --- a/compiler/qsc_passes/src/borrowck.rs +++ b/compiler/qsc_passes/src/borrowck.rs @@ -44,7 +44,7 @@ impl Checker { PatKind::Bind(ident) => { self.mutable.insert(ident.id); } - PatKind::Discard => {} + PatKind::Discard | PatKind::Err => {} PatKind::Tuple(tup) => { for pat in tup { self.track_pat(pat); diff --git a/compiler/qsc_passes/src/replace_qubit_allocation.rs b/compiler/qsc_passes/src/replace_qubit_allocation.rs index 59ec07d3e6..3a40e5ad00 100644 --- a/compiler/qsc_passes/src/replace_qubit_allocation.rs +++ b/compiler/qsc_passes/src/replace_qubit_allocation.rs @@ -54,6 +54,7 @@ impl<'a> ReplaceQubitAllocation<'a> { QubitInitKind::Array(e) => (true, Some(take(e))), QubitInitKind::Single => (true, None), QubitInitKind::Tuple(_) => (false, None), + QubitInitKind::Err => panic!("QubitInitKind::Err"), } } @@ -178,6 +179,7 @@ impl<'a> ReplaceQubitAllocation<'a> { }; (tuple_expr, ids) } + QubitInitKind::Err => panic!("QubitInitKind::Err"), } } @@ -448,6 +450,7 @@ fn create_qubit_global_alloc( .collect(), ), }, + QubitInitKind::Err => panic!("QubitInitKind::Err"), } } diff --git a/language_service/src/display.rs b/language_service/src/display.rs index 57d3464b8c..4e7e592bde 100644 --- a/language_service/src/display.rs +++ b/language_service/src/display.rs @@ -306,6 +306,7 @@ impl<'a> Display for HirPat<'a> { write!(f, "()") } } + hir::PatKind::Err => write!(f, "?"), } } } @@ -376,6 +377,7 @@ impl<'a> Display for AstPat<'a> { write!(f, "()") } } + ast::PatKind::Err => write!(f, "?"), } } } @@ -639,6 +641,7 @@ impl<'a> Display for AstTy<'a> { write!(f, ")") } } + ast::TyKind::Err => write!(f, "?"), } } } @@ -698,6 +701,7 @@ impl<'a> Display for TyDef<'a> { write!(f, ")") } } + ast::TyDefKind::Err => write!(f, "?"), } } } diff --git a/language_service/src/hover.rs b/language_service/src/hover.rs index ebf57babf4..aac43c4255 100644 --- a/language_service/src/hover.rs +++ b/language_service/src/hover.rs @@ -233,7 +233,7 @@ fn is_param(param_pats: &[&ast::Pat], node_id: ast::NodeId) -> bool { fn find_in_pat(pat: &ast::Pat, node_id: ast::NodeId) -> bool { match &*pat.kind { ast::PatKind::Bind(ident, _) => node_id == ident.id, - ast::PatKind::Discard(_) | ast::PatKind::Elided => false, + ast::PatKind::Discard(_) | ast::PatKind::Elided | ast::PatKind::Err => false, ast::PatKind::Paren(inner) => find_in_pat(inner, node_id), ast::PatKind::Tuple(inner) => inner.iter().any(|x| find_in_pat(x, node_id)), } diff --git a/language_service/src/signature_help.rs b/language_service/src/signature_help.rs index 37a9801f04..5faf29ddc6 100644 --- a/language_service/src/signature_help.rs +++ b/language_service/src/signature_help.rs @@ -145,7 +145,7 @@ impl SignatureHelpFinder<'_> { let mut offset = self.display.get_param_offset(package_id, decl); match &decl.input.kind { - hir::PatKind::Discard | hir::PatKind::Bind(_) => { + hir::PatKind::Discard | hir::PatKind::Err | hir::PatKind::Bind(_) => { self.make_wrapped_params(offset, package_id, &decl.input, doc) } hir::PatKind::Tuple(_) => { @@ -198,7 +198,7 @@ impl SignatureHelpFinder<'_> { doc: &str, ) -> Vec { match &pat.kind { - hir::PatKind::Bind(_) | hir::PatKind::Discard => { + hir::PatKind::Bind(_) | hir::PatKind::Discard | hir::PatKind::Err => { let documentation = if let hir::PatKind::Bind(name) = &pat.kind { let documentation = parse_doc_for_param(doc, &name.name); (!documentation.is_empty()).then_some(documentation) @@ -376,7 +376,7 @@ fn make_fake_pat(ty: &hir::ty::Ty) -> hir::Pat { fn process_args(args: &ast::Expr, location: u32, params: &hir::Pat) -> u32 { fn count_params(params: &hir::Pat) -> i32 { match ¶ms.kind { - hir::PatKind::Bind(_) | hir::PatKind::Discard => 1, + hir::PatKind::Bind(_) | hir::PatKind::Discard | hir::PatKind::Err => 1, hir::PatKind::Tuple(items) => items.iter().map(count_params).sum::() + 1, } } diff --git a/language_service/src/signature_help/tests.rs b/language_service/src/signature_help/tests.rs index 161e9b6236..4b991e9de7 100644 --- a/language_service/src/signature_help/tests.rs +++ b/language_service/src/signature_help/tests.rs @@ -244,7 +244,6 @@ fn last_argument() { ); } -#[ignore = "Parser needs updating to handle `(1,, \"Four\")`"] #[test] fn insert_second_argument() { check( @@ -257,7 +256,48 @@ fn insert_second_argument() { } } "#}, - &expect![[r#""#]], + &expect![[r#" + SignatureHelp { + signatures: [ + SignatureInformation { + label: "operation Foo(x : Int, y : Double, z : String) : Unit", + documentation: None, + parameters: [ + ParameterInformation { + label: Span { + start: 13, + end: 46, + }, + documentation: None, + }, + ParameterInformation { + label: Span { + start: 14, + end: 21, + }, + documentation: None, + }, + ParameterInformation { + label: Span { + start: 23, + end: 33, + }, + documentation: None, + }, + ParameterInformation { + label: Span { + start: 35, + end: 45, + }, + documentation: None, + }, + ], + }, + ], + active_signature: 0, + active_parameter: 2, + } + "#]], ); }