Check boolean condition on assert statement

Author: kraigher

vhdl_lang/src/analysis/expression.rs

@@ -533,6 +533,93 @@ impl<'a> AnalyzeContext<'a> {
         self.expr_pos_with_ttyp(scope, target_type, &expr.pos, &mut expr.item, diagnostics)
     }
 
+    fn implicit_bool_types(&self, scope: &Scope<'a>, pos: &SrcPos) -> FnvHashSet<BaseType<'a>> {
+        if let Ok(NamedEntities::Overloaded(overloaded)) =
+            scope.lookup(pos, &Designator::OperatorSymbol(Operator::QueQue))
+        {
+            overloaded
+                .entities()
+                .filter_map(|ent| ent.formals().nth(0).map(|typ| typ.type_mark().base()))
+                .collect()
+        } else {
+            FnvHashSet::default()
+        }
+    }
+
+    /// An expression that is either boolean or implicitly boolean via ?? operator
+    pub fn boolean_expr(
+        &self,
+        scope: &Scope<'a>,
+        expr: &mut WithPos<Expression>,
+        diagnostics: &mut dyn DiagnosticHandler,
+    ) -> FatalResult {
+        if let Some(types) = as_fatal(self.expr_type(scope, expr, diagnostics))? {
+            match types {
+                ExpressionType::Unambiguous(typ) => {
+                    if typ.base() != self.boolean().base() {
+                        let implicit_bools = self.implicit_bool_types(scope, &expr.pos);
+                        if !implicit_bools.contains(&typ.base()) {
+                            diagnostics.error(
+                                &expr.pos,
+                                format!(
+                                    "{} cannot be implictly converted to {}. Operator ?? is not defined for this type.",
+                                    typ.describe(),
+                                    self.boolean().describe()
+                                ),
+                            );
+                        }
+                    }
+                }
+                ExpressionType::Ambiguous(types) => {
+                    if types.contains(&self.boolean().base()) {
+                        self.expr_with_ttyp(scope, self.boolean(), expr, diagnostics)?;
+                    } else {
+                        let implicit_bool_types: FnvHashSet<_> = self
+                            .implicit_bool_types(scope, &expr.pos)
+                            .intersection(&types)
+                            .cloned()
+                            .collect();
+
+                        match implicit_bool_types.len().cmp(&1) {
+                            std::cmp::Ordering::Equal => {
+                                let typ: TypeEnt = types.into_iter().next().unwrap().into();
+                                self.expr_with_ttyp(scope, typ, expr, diagnostics)?;
+                            }
+                            std::cmp::Ordering::Greater => {
+                                let mut diag = Diagnostic::error(
+                                    &expr.pos,
+                                    "Ambiguous use of implicit boolean conversion ??",
+                                );
+                                diag.add_type_candididates("Could be", implicit_bool_types);
+                                diagnostics.push(diag);
+                            }
+
+                            std::cmp::Ordering::Less => {
+                                let mut diag = Diagnostic::error(
+                                    &expr.pos,
+                                    format!(
+                                        "Cannot disambiguate expression to {}",
+                                        self.boolean().describe()
+                                    ),
+                                );
+                                diag.add_type_candididates(
+                                    "Implicit boolean conversion operator ?? is not defined for",
+                                    types,
+                                );
+                                diagnostics.push(diag);
+                            }
+                        }
+                    }
+                }
+                ExpressionType::String | ExpressionType::Null | ExpressionType::Aggregate => {
+                    self.expr_with_ttyp(scope, self.boolean(), expr, diagnostics)?;
+                }
+            }
+        }
+
+        Ok(())
+    }
+
     /// Returns true if the name actually matches the target type
     /// None if it was uncertain
     pub fn expr_pos_with_ttyp(

vhdl_lang/src/analysis/formal_region.rs

@@ -142,10 +142,12 @@ impl<'a> FormalRegion<'a> {
         self.entities.get(idx).cloned()
     }
 
-    pub fn binary(&self) -> Option<(InterfaceEnt, InterfaceEnt<'a>)> {
-        let left = self.nth(0)?;
-        let right = self.nth(1)?;
-        Some((left, right))
+    pub fn unary(&self) -> Option<InterfaceEnt<'a>> {
+        if self.len() == 1 {
+            self.nth(0)
+        } else {
+            None
+        }
     }
 }
 

vhdl_lang/src/analysis/named_entity.rs

@@ -54,7 +54,7 @@ pub enum AnyEntKind<'a> {
 }
 
 impl<'a> AnyEntKind<'a> {
-    pub fn new_function_decl(
+    pub(crate) fn new_function_decl(
         formals: FormalRegion<'a>,
         return_type: TypeEnt<'a>,
     ) -> AnyEntKind<'a> {
@@ -64,7 +64,7 @@ impl<'a> AnyEntKind<'a> {
         )))
     }
 
-    pub fn new_procedure_decl(formals: FormalRegion<'a>) -> AnyEntKind<'a> {
+    pub(crate) fn new_procedure_decl(formals: FormalRegion<'a>) -> AnyEntKind<'a> {
         AnyEntKind::Overloaded(Overloaded::SubprogramDecl(Signature::new(formals, None)))
     }
 

vhdl_lang/src/analysis/semantic.rs

@@ -223,6 +223,21 @@ impl Diagnostic {
             }
         }
     }
+
+    pub fn add_type_candididates<'a>(
+        &mut self,
+        prefix: &str,
+        candidates: impl IntoIterator<Item = BaseType<'a>>,
+    ) {
+        let mut candidates: Vec<_> = candidates.into_iter().collect();
+        candidates.sort_by(|x, y| x.decl_pos().cmp(&y.decl_pos()));
+
+        for ent in candidates {
+            if let Some(decl_pos) = ent.decl_pos() {
+                self.add_related(decl_pos, format!("{} {}", prefix, ent.describe()))
+            }
+        }
+    }
 }
 
 impl<'a> AnyEnt<'a> {

vhdl_lang/src/analysis/sequential.rs

@@ -74,7 +74,7 @@ impl<'a> AnalyzeContext<'a> {
                     report,
                     severity,
                 } = assert_stmt;
-                self.analyze_expression(scope, condition, diagnostics)?;
+                self.boolean_expr(scope, condition, diagnostics)?;
                 if let Some(expr) = report {
                     self.expr_with_ttyp(scope, self.string(), expr, diagnostics)?;
                 }

vhdl_lang/src/analysis/tests/typecheck_expression.rs

@@ -1237,6 +1237,8 @@ begin
    assert true report 16#bad#;
    assert true report \"good\" severity error;
    assert true report \"good\" severity \"bad\";
+   assert 123;
+   assert bit'('0');
 end;
 ",
     );
@@ -1253,6 +1255,120 @@ end;
                 code.s1("\"bad\""),
                 "string literal does not match type 'SEVERITY_LEVEL'",
             ),
+            Diagnostic::error(
+                code.s1("123"),
+                "type universal_integer cannot be implictly converted to type 'BOOLEAN'. Operator ?? is not defined for this type.",
+            ),
         ],
     );
 }
+
+#[test]
+fn ambiguous_boolean_conversion_favors_boolean() {
+    let mut builder = LibraryBuilder::new();
+    let code = builder.in_declarative_region(
+        "
+function myfun return boolean is
+begin
+  return true;
+end function;
+
+function myfun return bit is
+begin
+  return '0';
+end function;
+
+procedure wrapper is
+begin
+   assert myfun;
+end;
+",
+    );
+
+    let (root, diagnostics) = builder.get_analyzed_root();
+    check_no_diagnostics(&diagnostics);
+
+    let decl = root
+        .search_reference(code.source(), code.s1("assert myfun;").s1("myfun").start())
+        .unwrap();
+
+    // Favors boolean
+    assert_eq!(
+        decl.decl_pos().unwrap(),
+        &code
+            .s1("function myfun return boolean is")
+            .s1("myfun")
+            .pos(),
+    );
+}
+
+#[test]
+fn ambiguous_qq_conversion() {
+    let mut builder = LibraryBuilder::new();
+    let code = builder.in_declarative_region(
+        "
+type typ1_t is (alpha, beta);
+type typ2_t is (alpha, beta);
+type typ3_t is (alpha, beta);
+
+function \"??\"(val : typ1_t) return boolean is
+begin
+  return true;
+end function;
+
+function \"??\"(val : typ2_t) return boolean is
+begin
+  return true;
+end function;
+
+procedure wrapper is
+begin
+   assert alpha;
+end;
+",
+    );
+
+    let diagnostics = builder.analyze();
+    check_diagnostics(
+        diagnostics,
+        vec![Diagnostic::error(
+            code.s1("assert alpha").s1("alpha"),
+            "Ambiguous use of implicit boolean conversion ??",
+        )
+        .related(code.s1("typ1_t"), "Could be type 'typ1_t'")
+        .related(code.s1("typ2_t"), "Could be type 'typ2_t'")],
+    );
+}
+
+#[test]
+fn ambiguous_qq_conversion_no_candidates() {
+    let mut builder = LibraryBuilder::new();
+    let code = builder.in_declarative_region(
+        "
+type typ1_t is (alpha, beta);
+type typ2_t is (alpha, beta);
+
+procedure wrapper is
+begin
+   assert alpha;
+end;
+",
+    );
+
+    let diagnostics = builder.analyze();
+    check_diagnostics(
+        diagnostics,
+        vec![Diagnostic::error(
+            code.s1("assert alpha").s1("alpha"),
+            "Cannot disambiguate expression to type 'BOOLEAN'",
+        )
+        .related(
+            code.s1("typ1_t"),
+            "Implicit boolean conversion operator ?? is not defined for type 'typ1_t'",
+        )
+        .related(
+            code.s1("typ2_t"),
+            "Implicit boolean conversion operator ?? is not defined for type 'typ2_t'",
+        )],
+    );
+}