Miscellaneous improvements (#319)

Do not discard the result of checking expressions
Simplify and canonicalises the AST of waveform assignments
Enables completions from within subprograms
Remove completions after ';' token

vhdl_lang/src/analysis/assignment.rs

@@ -60,13 +60,16 @@ impl<'a, 't> AnalyzeContext<'a, 't> {
     pub fn analyze_waveform_assignment(
         &self,
         scope: &Scope<'a>,
-        target: &mut WithTokenSpan<Target>,
-        assignment_type: AssignmentType,
-        rhs: &mut AssignmentRightHand<Waveform>,
+        assignment: &mut SignalAssignment,
         diagnostics: &mut dyn DiagnosticHandler,
     ) -> FatalResult {
-        let ttyp = as_fatal(self.resolve_target(scope, target, assignment_type, diagnostics))?;
-        match rhs {
+        let ttyp = as_fatal(self.resolve_target(
+            scope,
+            &mut assignment.target,
+            AssignmentType::Signal,
+            diagnostics,
+        ))?;
+        match &mut assignment.rhs {
             AssignmentRightHand::Simple(wavf) => {
                 self.analyze_waveform(scope, ttyp, wavf, diagnostics)?;
             }

vhdl_lang/src/analysis/concurrent.rs

@@ -13,7 +13,6 @@ use crate::data::*;
 use crate::named_entity::*;
 use crate::{HasTokenSpan, TokenSpan};
 use analyze::*;
-use target::AssignmentType;
 
 impl<'a, 't> AnalyzeContext<'a, 't> {
     pub fn analyze_concurrent_part(
@@ -198,14 +197,8 @@ impl<'a, 't> AnalyzeContext<'a, 't> {
             }
             ConcurrentStatement::Assignment(ref mut assign) => {
                 // @TODO more delaymechanism
-                let ConcurrentSignalAssignment { target, rhs, .. } = assign;
-                self.analyze_waveform_assignment(
-                    scope,
-                    target,
-                    AssignmentType::Signal,
-                    rhs,
-                    diagnostics,
-                )?;
+                let ConcurrentSignalAssignment { assignment, .. } = assign;
+                self.analyze_waveform_assignment(scope, assignment, diagnostics)?;
             }
             ConcurrentStatement::ProcedureCall(ref mut pcall) => {
                 let ConcurrentProcedureCall { call, .. } = pcall;

vhdl_lang/src/analysis/expression.rs

@@ -300,18 +300,9 @@ impl<'a, 't> AnalyzeContext<'a, 't> {
         diagnostics: &mut dyn DiagnosticHandler,
     ) -> EvalResult<Vec<ExpressionType<'a>>> {
         let mut operand_types = Vec::with_capacity(operands.len());
-
-        let mut expr_diagnostics = Vec::new();
         for expr in operands.iter_mut() {
-            if let Some(types) = as_fatal(self.expr_type(scope, expr, &mut expr_diagnostics))? {
-                operand_types.push(types);
-            } else {
-                // bail if any operator argument is unknown
-                diagnostics.append(expr_diagnostics);
-                return Err(EvalError::Unknown);
-            }
+            operand_types.push(self.expr_type(scope, expr, diagnostics)?);
         }
-
         Ok(operand_types)
     }
 

vhdl_lang/src/analysis/sequential.rs

@@ -287,15 +287,7 @@ impl<'a, 't> AnalyzeContext<'a, 't> {
                 self.analyze_procedure_call(scope, pcall, diagnostics)?;
             }
             SequentialStatement::SignalAssignment(ref mut assign) => {
-                // @TODO more
-                let SignalAssignment { target, rhs, .. } = assign;
-                self.analyze_waveform_assignment(
-                    scope,
-                    target,
-                    AssignmentType::Signal,
-                    rhs,
-                    diagnostics,
-                )?;
+                self.analyze_waveform_assignment(scope, assign, diagnostics)?;
             }
             SequentialStatement::VariableAssignment(ref mut assign) => {
                 let VariableAssignment { target, rhs } = assign;

vhdl_lang/src/analysis/tests/typecheck_expression.rs

@@ -5,6 +5,7 @@
 // Copyright (c) 2019, Olof Kraigher olof.kraigher@gmail.com
 
 use super::*;
+use std::vec;
 use vhdl_lang::data::error_codes::ErrorCode;
 
 #[test]
@@ -1969,3 +1970,27 @@ end package;
     let diagnostics = builder.analyze();
     check_no_diagnostics(&diagnostics);
 }
+
+// Issue #317
+#[test]
+fn type_mismatch_in_binary_expression() {
+    let mut builder = LibraryBuilder::new();
+    let code = builder.code(
+        "libname",
+        "\
+package foo is
+    function takes_slv(din : bit_vector) return boolean;
+    constant bar : boolean := takes_slv(true) and true;
+end package;",
+    );
+
+    let diagnostics = builder.analyze();
+    check_diagnostics(
+        diagnostics,
+        vec![Diagnostic::new(
+            code.s1("true"),
+            "'true' does not match array type 'BIT_VECTOR'",
+            ErrorCode::TypeMismatch,
+        )],
+    );
+}

vhdl_lang/src/ast.rs

@@ -1102,9 +1102,7 @@ pub struct ConcurrentAssertStatement {
 pub struct ConcurrentSignalAssignment {
     pub postponed: bool,
     pub guarded: bool,
-    pub target: WithTokenSpan<Target>,
-    pub delay_mechanism: Option<DelayMechanism>,
-    pub rhs: AssignmentRightHand<Waveform>,
+    pub assignment: SignalAssignment,
 }
 
 /// 11.7 Component instantiation statements

vhdl_lang/src/ast/search.rs

@@ -522,8 +522,13 @@ impl Search for LabeledConcurrentStatement {
                 return_if_found!(inst.search(ctx, searcher));
             }
             ConcurrentStatement::Assignment(ref assign) => {
-                let ConcurrentSignalAssignment { target, rhs, .. } = assign;
-                return_if_found!(search_assignment(target, rhs, searcher, ctx));
+                let ConcurrentSignalAssignment { assignment, .. } = assign;
+                return_if_found!(search_assignment(
+                    &assignment.target,
+                    &assignment.rhs,
+                    searcher,
+                    ctx
+                ));
             }
             ConcurrentStatement::ProcedureCall(ref pcall) => {
                 let ConcurrentProcedureCall {

vhdl_lang/src/completion.rs

@@ -79,6 +79,11 @@ pub fn list_completion_options<'a>(
     use crate::syntax::Kind::*;
     let tokens = tokenize_input(root.symbols(), source, cursor);
     match &tokens[..] {
+        // With the current implementation of completions, this is annoying, rather than helpful.
+        // SemiColons will try to complete the ';' character, which when pressing enter will cause
+        // ';' to appear instead of a simple ; character.
+        // Therefore, we do not return any completions here.
+        [.., kind!(SemiColon)] => vec![],
         [.., kind!(Library)]
         | [.., kind!(Library), kind!(Identifier)]
         | [.., kind!(Use)]

vhdl_lang/src/completion/generic.rs

@@ -8,7 +8,7 @@ use crate::ast::ArchitectureBody;
 use crate::completion::entity_instantiation::get_visible_entities_from_architecture;
 use crate::completion::region::completion_items_from_region;
 use crate::named_entity::{DesignEnt, Visibility};
-use crate::{CompletionItem, Design, HasTokenSpan, Position, Source, TokenAccess};
+use crate::{CompletionItem, Design, HasEntityId, HasTokenSpan, Position, Source, TokenAccess};
 use itertools::{chain, Itertools};
 use vhdl_lang::analysis::DesignRoot;
 
@@ -98,6 +98,19 @@ impl<'a> Searcher for CompletionSearcher<'a> {
                 }
                 package.ident.decl.get()
             }
+            FoundDeclaration::Subprogram(subprogram) => {
+                if !subprogram.get_pos(ctx).contains(self.cursor) {
+                    return NotFinished;
+                }
+                self.completions.extend(
+                    subprogram
+                        .declarations
+                        .iter()
+                        .flat_map(|decl| decl.item.ent_id())
+                        .map(|id| CompletionItem::Simple(self.root.get_ent(id))),
+                );
+                return NotFinished;
+            }
             _ => return NotFinished,
         };
         let Some(ent_id) = ent_id else {
@@ -108,7 +121,7 @@ impl<'a> Searcher for CompletionSearcher<'a> {
         };
         self.completions
             .extend(visible_entities_from(self.root, ent.kind()));
-        Finished(Found)
+        NotFinished
     }
 }
 

vhdl_lang/src/syntax/concurrent_statement.rs

@@ -283,13 +283,16 @@ fn parse_assignment_known_target(
     // @TODO guarded
     let guarded = false;
     let delay_mechanism = parse_delay_mechanism(ctx)?;
+    let rhs = parse_signal_assignment_right_hand(ctx)?;
     Ok(ConcurrentStatement::Assignment(
         ConcurrentSignalAssignment {
             postponed,
             guarded,
-            target,
-            delay_mechanism,
-            rhs: parse_signal_assignment_right_hand(ctx)?,
+            assignment: SignalAssignment {
+                target,
+                delay_mechanism,
+                rhs,
+            },
         },
     ))
 }
@@ -323,9 +326,11 @@ fn parse_selected_signal_assignment(
     Ok(ConcurrentSignalAssignment {
         postponed,
         guarded,
-        target,
-        delay_mechanism,
-        rhs,
+        assignment: SignalAssignment {
+            target,
+            delay_mechanism,
+            rhs,
+        },
     })
 }
 
@@ -1245,9 +1250,11 @@ end process;",
         let assign = ConcurrentSignalAssignment {
             postponed: false,
             guarded: false,
-            target: code.s1("foo").name().map_into(Target::Name),
-            delay_mechanism: None,
-            rhs: AssignmentRightHand::Simple(code.s1("bar(2 to 3)").waveform()),
+            assignment: SignalAssignment {
+                target: code.s1("foo").name().map_into(Target::Name),
+                delay_mechanism: None,
+                rhs: AssignmentRightHand::Simple(code.s1("bar(2 to 3)").waveform()),
+            },
         };
         let stmt = code.with_stream_no_diagnostics(parse_labeled_concurrent_statement);
         assert_eq!(stmt.label.tree, None);
@@ -1263,12 +1270,14 @@ end process;",
         let assign = ConcurrentSignalAssignment {
             postponed: false,
             guarded: false,
-            target: code
-                .s1("<< signal dut.foo : std_logic >>")
-                .name()
-                .map_into(Target::Name),
-            delay_mechanism: None,
-            rhs: AssignmentRightHand::Simple(code.s1("bar(2 to 3)").waveform()),
+            assignment: SignalAssignment {
+                target: code
+                    .s1("<< signal dut.foo : std_logic >>")
+                    .name()
+                    .map_into(Target::Name),
+                delay_mechanism: None,
+                rhs: AssignmentRightHand::Simple(code.s1("bar(2 to 3)").waveform()),
+            },
         };
         let stmt = code.with_stream_no_diagnostics(parse_labeled_concurrent_statement);
         assert_eq!(stmt.label.tree, None);
@@ -1301,9 +1310,11 @@ with x(0) + 1 select
             ConcurrentStatement::Assignment(ConcurrentSignalAssignment {
                 postponed: false,
                 guarded: false,
-                target: code.s1("foo(0)").name().map_into(Target::Name),
-                delay_mechanism: Some(DelayMechanism::Transport),
-                rhs: AssignmentRightHand::Selected(selection)
+                assignment: SignalAssignment {
+                    target: code.s1("foo(0)").name().map_into(Target::Name),
+                    delay_mechanism: Some(DelayMechanism::Transport),
+                    rhs: AssignmentRightHand::Selected(selection)
+                }
             })
         );
         assert_eq!(stmt.statement.span, code.token_span());

vhdl_lang/src/syntax/sequential_statement.rs

@@ -530,8 +530,8 @@ fn parse_selected_assignment(ctx: &mut ParsingContext<'_>) -> ParseResult<Sequen
                     target,
                     delay_mechanism: parse_delay_mechanism(ctx)?,
                     rhs: AssignmentRightHand::Selected(parse_selection(ctx, expression, parse_waveform)?)
-            }))
-        }
+                }))
+            }
         }
     )
 }