Better incompatible-dimensions errors

examples/typecheck_error/incompatible_alternative_dimensions.nbt

@@ -0,0 +1,2 @@
+dimension MyEnergy = Momentum^2 / Mass = Mass × Speed^2 = Force × Length^2
+

numbat/src/ast.rs

@@ -302,25 +302,43 @@ impl PrettyPrint for Expression {
 #[derive(Debug, Clone, PartialEq)]
 
 pub enum DimensionExpression {
-    Unity,
-    Dimension(String),
-    Multiply(Box<DimensionExpression>, Box<DimensionExpression>),
-    Divide(Box<DimensionExpression>, Box<DimensionExpression>),
-    Power(Box<DimensionExpression>, Exponent),
+    Unity(Span),
+    Dimension(Span, String),
+    Multiply(Span, Box<DimensionExpression>, Box<DimensionExpression>),
+    Divide(Span, Box<DimensionExpression>, Box<DimensionExpression>),
+    Power(Span, Box<DimensionExpression>, Span, Exponent),
+}
+
+impl DimensionExpression {
+    pub fn full_span(&self) -> Span {
+        match self {
+            DimensionExpression::Unity(s) => *s,
+            DimensionExpression::Dimension(s, _) => *s,
+            DimensionExpression::Multiply(span_op, lhs, rhs) => {
+                span_op.extend(&lhs.full_span()).extend(&rhs.full_span())
+            }
+            DimensionExpression::Divide(span_op, lhs, rhs) => {
+                span_op.extend(&lhs.full_span()).extend(&rhs.full_span())
+            }
+            DimensionExpression::Power(span_op, lhs, span_exponent, _exp) => {
+                span_op.extend(&lhs.full_span()).extend(&span_exponent)
+            }
+        }
+    }
 }
 
 impl PrettyPrint for DimensionExpression {
     fn pretty_print(&self) -> Markup {
         match self {
-            DimensionExpression::Unity => m::type_identifier("1"),
-            DimensionExpression::Dimension(ident) => m::type_identifier(ident),
-            DimensionExpression::Multiply(lhs, rhs) => {
+            DimensionExpression::Unity(_) => m::type_identifier("1"),
+            DimensionExpression::Dimension(_, ident) => m::type_identifier(ident),
+            DimensionExpression::Multiply(_, lhs, rhs) => {
                 lhs.pretty_print() + m::space() + m::operator("×") + m::space() + rhs.pretty_print()
             }
-            DimensionExpression::Divide(lhs, rhs) => {
+            DimensionExpression::Divide(_, lhs, rhs) => {
                 lhs.pretty_print() + m::space() + m::operator("/") + m::space() + rhs.pretty_print()
             }
-            DimensionExpression::Power(lhs, exp) => {
+            DimensionExpression::Power(_, lhs, _, exp) => {
                 m::operator("(")
                     + lhs.pretty_print()
                     + m::operator(")")
@@ -586,6 +604,34 @@ pub trait ReplaceSpans {
     fn replace_spans(&self) -> Self;
 }
 
+#[cfg(test)]
+impl ReplaceSpans for DimensionExpression {
+    fn replace_spans(&self) -> Self {
+        match self {
+            DimensionExpression::Unity(_) => DimensionExpression::Unity(Span::dummy()),
+            DimensionExpression::Dimension(_, d) => {
+                DimensionExpression::Dimension(Span::dummy(), d.clone())
+            }
+            DimensionExpression::Multiply(_, lhs, rhs) => DimensionExpression::Multiply(
+                Span::dummy(),
+                Box::new(lhs.replace_spans()),
+                Box::new(rhs.replace_spans()),
+            ),
+            DimensionExpression::Divide(_, lhs, rhs) => DimensionExpression::Divide(
+                Span::dummy(),
+                Box::new(lhs.replace_spans()),
+                Box::new(rhs.replace_spans()),
+            ),
+            DimensionExpression::Power(_, lhs, _, exp) => DimensionExpression::Power(
+                Span::dummy(),
+                Box::new(lhs.replace_spans()),
+                Span::dummy(),
+                exp.clone(),
+            ),
+        }
+    }
+}
+
 #[cfg(test)]
 impl ReplaceSpans for Expression {
     fn replace_spans(&self) -> Self {
@@ -637,7 +683,7 @@ impl ReplaceSpans for Statement {
                 identifier: identifier.clone(),
                 expr: expr.replace_spans(),
                 type_annotation_span: type_annotation_span.map(|_| Span::dummy()),
-                type_annotation: type_annotation.clone(),
+                type_annotation: type_annotation.as_ref().map(|t| t.replace_spans()),
             },
             Statement::DeclareFunction {
                 function_name_span: _,
@@ -653,17 +699,27 @@ impl ReplaceSpans for Statement {
                 type_parameters: type_parameters.clone(),
                 parameters: parameters
                     .iter()
-                    .map(|(_, a, b, c)| (Span::dummy(), a.clone(), b.clone(), c.clone()))
+                    .map(|(_, name, type_, is_variadic)| {
+                        (
+                            Span::dummy(),
+                            name.clone(),
+                            type_.as_ref().map(|t| t.replace_spans()),
+                            *is_variadic,
+                        )
+                    })
                     .collect(),
                 body: body.clone().map(|b| b.replace_spans()),
                 return_type_span: return_type_span.map(|_| Span::dummy()),
-                return_type_annotation: return_type_annotation.clone(),
+                return_type_annotation: return_type_annotation.as_ref().map(|t| t.replace_spans()),
             },
-            s @ Statement::DeclareDimension(_, _) => s.clone(),
+            Statement::DeclareDimension(name, dexprs) => Statement::DeclareDimension(
+                name.clone(),
+                dexprs.iter().map(|t| t.replace_spans()).collect(),
+            ),
             Statement::DeclareBaseUnit(_, name, type_, decorators) => Statement::DeclareBaseUnit(
                 Span::dummy(),
                 name.clone(),
-                type_.clone(),
+                type_.replace_spans(),
                 decorators.clone(),
             ),
             Statement::DeclareDerivedUnit {
@@ -678,7 +734,7 @@ impl ReplaceSpans for Statement {
                 identifier: identifier.clone(),
                 expr: expr.replace_spans(),
                 type_annotation_span: type_annotation_span.map(|_| Span::dummy()),
-                type_annotation: type_annotation.clone(),
+                type_annotation: type_annotation.as_ref().map(|t| t.replace_spans()),
                 decorators: decorators.clone(),
             },
             Statement::ProcedureCall(_, proc, args) => Statement::ProcedureCall(

numbat/src/diagnostic.rs

@@ -96,9 +96,22 @@ impl ErrorDiagnostic for TypeCheckError {
                 .diagnostic_label(LabelStyle::Primary)
                 .with_message(inner_error)]),
             TypeCheckError::RegistryError(_) => d.with_notes(vec![inner_error]),
-            TypeCheckError::IncompatibleAlternativeDimensionExpression(_) => {
-                d.with_notes(vec![inner_error])
-            }
+            TypeCheckError::IncompatibleAlternativeDimensionExpression(
+                _name,
+                span1,
+                type1,
+                span2,
+                type2,
+            ) => d
+                .with_labels(vec![
+                    span1
+                        .diagnostic_label(LabelStyle::Primary)
+                        .with_message(type1.to_string()),
+                    span2
+                        .diagnostic_label(LabelStyle::Primary)
+                        .with_message(type2.to_string()),
+                ])
+                .with_notes(vec![inner_error]),
             TypeCheckError::WrongArity {
                 callable_span,
                 callable_name: _,

numbat/src/dimension.rs

@@ -13,23 +13,23 @@ impl DimensionRegistry {
         expression: &DimensionExpression,
     ) -> Result<BaseRepresentation> {
         match expression {
-            DimensionExpression::Unity => Ok(BaseRepresentation::unity()),
-            DimensionExpression::Dimension(name) => {
+            DimensionExpression::Unity(_) => Ok(BaseRepresentation::unity()),
+            DimensionExpression::Dimension(_, name) => {
                 self.registry.get_base_representation_for_name(name)
             }
-            DimensionExpression::Multiply(lhs, rhs) => {
+            DimensionExpression::Multiply(_, lhs, rhs) => {
                 let lhs = self.get_base_representation(lhs)?;
                 let rhs = self.get_base_representation(rhs)?;
 
                 Ok(lhs * rhs)
             }
-            DimensionExpression::Divide(lhs, rhs) => {
+            DimensionExpression::Divide(_, lhs, rhs) => {
                 let lhs = self.get_base_representation(lhs)?;
                 let rhs = self.get_base_representation(rhs)?;
 
                 Ok(lhs / rhs)
             }
-            DimensionExpression::Power(expr, outer_exponent) => {
+            DimensionExpression::Power(_, expr, _, outer_exponent) => {
                 Ok(self.get_base_representation(expr)?.power(*outer_exponent))
             }
         }

numbat/src/parser.rs

@@ -846,12 +846,13 @@ impl<'a> Parser<'a> {
     fn dimension_factor(&mut self) -> Result<DimensionExpression> {
         let mut expr = self.dimension_power()?;
         while let Some(operator_token) = self.match_any(&[TokenKind::Multiply, TokenKind::Divide]) {
+            let span = self.last().unwrap().span;
             let rhs = self.dimension_power()?;
 
             expr = if operator_token.kind == TokenKind::Multiply {
-                DimensionExpression::Multiply(Box::new(expr), Box::new(rhs))
+                DimensionExpression::Multiply(span, Box::new(expr), Box::new(rhs))
             } else {
-                DimensionExpression::Divide(Box::new(expr), Box::new(rhs))
+                DimensionExpression::Divide(span, Box::new(expr), Box::new(rhs))
             };
         }
         Ok(expr)
@@ -861,29 +862,44 @@ impl<'a> Parser<'a> {
         let expr = self.dimension_primary()?;
 
         if self.match_exact(TokenKind::Power).is_some() {
-            let exponent = self.dimension_exponent()?;
+            let span = self.last().unwrap().span;
+            let (span_exponent, exponent) = self.dimension_exponent()?;
 
-            Ok(DimensionExpression::Power(Box::new(expr), exponent))
+            Ok(DimensionExpression::Power(
+                span,
+                Box::new(expr),
+                span_exponent,
+                exponent,
+            ))
         } else {
             Ok(expr)
         }
     }
 
-    fn dimension_exponent(&mut self) -> Result<Exponent> {
+    fn dimension_exponent(&mut self) -> Result<(Span, Exponent)> {
         // TODO: potentially allow for ², ³, etc.
 
         if let Some(token) = self.match_exact(TokenKind::Number) {
+            let span = self.last().unwrap().span;
             // TODO: only parse integers here
-            Ok(Rational::from_f64(token.lexeme.parse::<f64>().unwrap()).unwrap())
+            Ok((
+                span,
+                Rational::from_f64(token.lexeme.parse::<f64>().unwrap()).unwrap(),
+            ))
         } else if self.match_exact(TokenKind::Minus).is_some() {
-            let exponent = self.dimension_exponent()?;
-            Ok(-exponent)
+            let span = self.last().unwrap().span;
+            let (span_inner, exponent) = self.dimension_exponent()?;
+            Ok((span.extend(&span_inner), -exponent))
         } else if self.match_exact(TokenKind::LeftParen).is_some() {
-            let exponent = self.dimension_exponent()?;
+            let mut span = self.last().unwrap().span;
+            let (span_inner, exponent) = self.dimension_exponent()?;
+            span = span.extend(&span_inner);
             if self.match_exact(TokenKind::RightParen).is_some() {
-                Ok(exponent)
+                span = span.extend(&self.last().unwrap().span);
+                Ok((span, exponent))
             } else if self.match_exact(TokenKind::Divide).is_some() {
-                let rhs = self.dimension_exponent()?;
+                let (span_rhs, rhs) = self.dimension_exponent()?;
+                span = span.extend(&span_rhs);
                 if rhs == Rational::zero() {
                     Err(ParseError::new(
                         ParseErrorKind::DivisionByZeroInDimensionExponent,
@@ -895,7 +911,8 @@ impl<'a> Parser<'a> {
                         self.peek().span,
                     ))
                 } else {
-                    Ok(exponent / rhs)
+                    span = span.extend(&self.last().unwrap().span);
+                    Ok((span, exponent / rhs))
                 }
             } else {
                 Err(ParseError::new(
@@ -914,12 +931,14 @@ impl<'a> Parser<'a> {
             self.peek().span,
         ));
         if let Some(token) = self.match_exact(TokenKind::Identifier) {
-            Ok(DimensionExpression::Dimension(token.lexeme.clone()))
+            let span = self.last().unwrap().span;
+            Ok(DimensionExpression::Dimension(span, token.lexeme.clone()))
         } else if let Some(number) = self.match_exact(TokenKind::Number) {
+            let span = self.last().unwrap().span;
             if number.lexeme != "1" {
                 e
             } else {
-                Ok(DimensionExpression::Unity)
+                Ok(DimensionExpression::Unity(span))
             }
         } else if self.match_exact(TokenKind::LeftParen).is_some() {
             let dexpr = self.dimension_expression()?;
@@ -1289,7 +1308,10 @@ mod tests {
                 identifier: "x".into(),
                 expr: binop!(scalar!(1.0), Mul, identifier!("meter")),
                 type_annotation_span: Some(Span::dummy()),
-                type_annotation: Some(DimensionExpression::Dimension("Length".into())),
+                type_annotation: Some(DimensionExpression::Dimension(
+                    Span::dummy(),
+                    "Length".into(),
+                )),
             },
         );
 
@@ -1319,8 +1341,15 @@ mod tests {
             Statement::DeclareDimension(
                 "Area".into(),
                 vec![DimensionExpression::Multiply(
-                    Box::new(DimensionExpression::Dimension("Length".into())),
-                    Box::new(DimensionExpression::Dimension("Length".into())),
+                    Span::dummy(),
+                    Box::new(DimensionExpression::Dimension(
+                        Span::dummy(),
+                        "Length".into(),
+                    )),
+                    Box::new(DimensionExpression::Dimension(
+                        Span::dummy(),
+                        "Length".into(),
+                    )),
                 )],
             ),
         );
@@ -1330,8 +1359,12 @@ mod tests {
             Statement::DeclareDimension(
                 "Speed".into(),
                 vec![DimensionExpression::Divide(
-                    Box::new(DimensionExpression::Dimension("Length".into())),
-                    Box::new(DimensionExpression::Dimension("Time".into())),
+                    Span::dummy(),
+                    Box::new(DimensionExpression::Dimension(
+                        Span::dummy(),
+                        "Length".into(),
+                    )),
+                    Box::new(DimensionExpression::Dimension(Span::dummy(), "Time".into())),
                 )],
             ),
         );
@@ -1341,7 +1374,12 @@ mod tests {
             Statement::DeclareDimension(
                 "Area".into(),
                 vec![DimensionExpression::Power(
-                    Box::new(DimensionExpression::Dimension("Length".into())),
+                    Span::dummy(),
+                    Box::new(DimensionExpression::Dimension(
+                        Span::dummy(),
+                        "Length".into(),
+                    )),
+                    Span::dummy(),
                     Rational::from_integer(2),
                 )],
             ),
@@ -1352,15 +1390,24 @@ mod tests {
             Statement::DeclareDimension(
                 "Energy".into(),
                 vec![DimensionExpression::Divide(
+                    Span::dummy(),
                     Box::new(DimensionExpression::Multiply(
-                        Box::new(DimensionExpression::Dimension("Mass".into())),
+                        Span::dummy(),
+                        Box::new(DimensionExpression::Dimension(Span::dummy(), "Mass".into())),
                         Box::new(DimensionExpression::Power(
-                            Box::new(DimensionExpression::Dimension("Length".into())),
+                            Span::dummy(),
+                            Box::new(DimensionExpression::Dimension(
+                                Span::dummy(),
+                                "Length".into(),
+                            )),
+                            Span::dummy(),
                             Rational::from_integer(2),
                         )),
                     )),
                     Box::new(DimensionExpression::Power(
-                        Box::new(DimensionExpression::Dimension("Time".into())),
+                        Span::dummy(),
+                        Box::new(DimensionExpression::Dimension(Span::dummy(), "Time".into())),
+                        Span::dummy(),
                         Rational::from_integer(2),
                     )),
                 )],
@@ -1392,7 +1439,10 @@ mod tests {
                 parameters: vec![],
                 body: Some(scalar!(1.0)),
                 return_type_span: Some(Span::dummy()),
-                return_type_annotation: Some(DimensionExpression::Dimension("Scalar".into())),
+                return_type_annotation: Some(DimensionExpression::Dimension(
+                    Span::dummy(),
+                    "Scalar".into(),
+                )),
             },
         );
 
@@ -1436,25 +1486,39 @@ mod tests {
                     (
                         Span::dummy(),
                         "x".into(),
-                        Some(DimensionExpression::Dimension("Length".into())),
+                        Some(DimensionExpression::Dimension(
+                            Span::dummy(),
+                            "Length".into(),
+                        )),
                         false,
                     ),
                     (
                         Span::dummy(),
                         "y".into(),
-                        Some(DimensionExpression::Dimension("Time".into())),
+                        Some(DimensionExpression::Dimension(Span::dummy(), "Time".into())),
                         false,
                     ),
                     (
                         Span::dummy(),
                         "z".into(),
                         Some(DimensionExpression::Multiply(
+                            Span::dummy(),
                             Box::new(DimensionExpression::Power(
-                                Box::new(DimensionExpression::Dimension("Length".into())),
+                                Span::dummy(),
+                                Box::new(DimensionExpression::Dimension(
+                                    Span::dummy(),
+                                    "Length".into(),
+                                )),
+                                Span::dummy(),
                                 Rational::new(3, 1),
                             )),
                             Box::new(DimensionExpression::Power(
-                                Box::new(DimensionExpression::Dimension("Time".into())),
+                                Span::dummy(),
+                                Box::new(DimensionExpression::Dimension(
+                                    Span::dummy(),
+                                    "Time".into(),
+                                )),
+                                Span::dummy(),
                                 Rational::new(2, 1),
                             )),
                         )),
@@ -1463,7 +1527,10 @@ mod tests {
                 ],
                 body: Some(scalar!(1.0)),
                 return_type_span: Some(Span::dummy()),
-                return_type_annotation: Some(DimensionExpression::Dimension("Scalar".into())),
+                return_type_annotation: Some(DimensionExpression::Dimension(
+                    Span::dummy(),
+                    "Scalar".into(),
+                )),
             },
         );
 
@@ -1476,7 +1543,7 @@ mod tests {
                 parameters: vec![(
                     Span::dummy(),
                     "x".into(),
-                    Some(DimensionExpression::Dimension("X".into())),
+                    Some(DimensionExpression::Dimension(Span::dummy(), "X".into())),
                     false,
                 )],
                 body: Some(scalar!(1.0)),
@@ -1494,12 +1561,15 @@ mod tests {
                 parameters: vec![(
                     Span::dummy(),
                     "x".into(),
-                    Some(DimensionExpression::Dimension("D".into())),
+                    Some(DimensionExpression::Dimension(Span::dummy(), "D".into())),
                     true,
                 )],
                 body: None,
                 return_type_span: Some(Span::dummy()),
-                return_type_annotation: Some(DimensionExpression::Dimension("D".into())),
+                return_type_annotation: Some(DimensionExpression::Dimension(
+                    Span::dummy(),
+                    "D".into(),
+                )),
             },
         );
 

numbat/src/typechecker.rs

@@ -45,7 +45,7 @@ pub enum TypeCheckError {
     RegistryError(RegistryError),
 
     #[error("Incompatible alternative expressions have been provided for dimension '{0}'")]
-    IncompatibleAlternativeDimensionExpression(String), // TODO: add span information
+    IncompatibleAlternativeDimensionExpression(String, Span, Type, Span, Type),
 
     #[error("Function or procedure '{callable_name}' called with {num_args} arguments(s), but needs {}..{}", arity.start(), arity.end())]
     WrongArity {
@@ -600,6 +600,10 @@ impl TypeChecker {
                             return Err(
                                 TypeCheckError::IncompatibleAlternativeDimensionExpression(
                                     name.clone(),
+                                    dexpr.full_span(),
+                                    base_representation,
+                                    alternative_expr.full_span(),
+                                    alternative_base_representation,
                                 ),
                             );
                         }
@@ -921,7 +925,7 @@ mod tests {
                 "# wrong alternative expression: C / B^2
                  dimension D = A / B = C / B^3"
             ),
-            TypeCheckError::IncompatibleAlternativeDimensionExpression(t) if t == "D",
+            TypeCheckError::IncompatibleAlternativeDimensionExpression(t, ..) if t == "D",
         ));
     }