From 1027cb22e67f9337a152573dd051e4a728fc8554 Mon Sep 17 00:00:00 2001 From: Lukas Scheller <45085299+Schottkyc137@users.noreply.github.com> Date: Sun, 14 Jul 2024 16:47:35 +0200 Subject: [PATCH] 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 | 13 +++-- vhdl_lang/src/analysis/concurrent.rs | 11 +---- vhdl_lang/src/analysis/expression.rs | 11 +---- vhdl_lang/src/analysis/sequential.rs | 10 +--- .../analysis/tests/typecheck_expression.rs | 25 ++++++++++ vhdl_lang/src/ast.rs | 4 +- vhdl_lang/src/ast/search.rs | 9 +++- vhdl_lang/src/completion.rs | 5 ++ vhdl_lang/src/completion/generic.rs | 17 ++++++- vhdl_lang/src/syntax/concurrent_statement.rs | 47 ++++++++++++------- vhdl_lang/src/syntax/sequential_statement.rs | 4 +- 11 files changed, 96 insertions(+), 60 deletions(-) diff --git a/vhdl_lang/src/analysis/assignment.rs b/vhdl_lang/src/analysis/assignment.rs index 5d5ef89c..df4883a8 100644 --- a/vhdl_lang/src/analysis/assignment.rs +++ b/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, - assignment_type: AssignmentType, - rhs: &mut AssignmentRightHand, + 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)?; } diff --git a/vhdl_lang/src/analysis/concurrent.rs b/vhdl_lang/src/analysis/concurrent.rs index 7d03ff37..1bc1cf3f 100644 --- a/vhdl_lang/src/analysis/concurrent.rs +++ b/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; diff --git a/vhdl_lang/src/analysis/expression.rs b/vhdl_lang/src/analysis/expression.rs index 776f6c96..386b8685 100644 --- a/vhdl_lang/src/analysis/expression.rs +++ b/vhdl_lang/src/analysis/expression.rs @@ -300,18 +300,9 @@ impl<'a, 't> AnalyzeContext<'a, 't> { diagnostics: &mut dyn DiagnosticHandler, ) -> EvalResult>> { 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) } diff --git a/vhdl_lang/src/analysis/sequential.rs b/vhdl_lang/src/analysis/sequential.rs index b9b93940..1887d7f6 100644 --- a/vhdl_lang/src/analysis/sequential.rs +++ b/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; diff --git a/vhdl_lang/src/analysis/tests/typecheck_expression.rs b/vhdl_lang/src/analysis/tests/typecheck_expression.rs index 1574eeed..3294d03e 100644 --- a/vhdl_lang/src/analysis/tests/typecheck_expression.rs +++ b/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, + )], + ); +} diff --git a/vhdl_lang/src/ast.rs b/vhdl_lang/src/ast.rs index a2d3b699..97c50c33 100644 --- a/vhdl_lang/src/ast.rs +++ b/vhdl_lang/src/ast.rs @@ -1102,9 +1102,7 @@ pub struct ConcurrentAssertStatement { pub struct ConcurrentSignalAssignment { pub postponed: bool, pub guarded: bool, - pub target: WithTokenSpan, - pub delay_mechanism: Option, - pub rhs: AssignmentRightHand, + pub assignment: SignalAssignment, } /// 11.7 Component instantiation statements diff --git a/vhdl_lang/src/ast/search.rs b/vhdl_lang/src/ast/search.rs index f821d133..7c95dff0 100644 --- a/vhdl_lang/src/ast/search.rs +++ b/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 { diff --git a/vhdl_lang/src/completion.rs b/vhdl_lang/src/completion.rs index 2106805a..5304da1d 100644 --- a/vhdl_lang/src/completion.rs +++ b/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)] diff --git a/vhdl_lang/src/completion/generic.rs b/vhdl_lang/src/completion/generic.rs index e3cc21bb..b74fa016 100644 --- a/vhdl_lang/src/completion/generic.rs +++ b/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 } } diff --git a/vhdl_lang/src/syntax/concurrent_statement.rs b/vhdl_lang/src/syntax/concurrent_statement.rs index ba3f1e18..4bfc97db 100644 --- a/vhdl_lang/src/syntax/concurrent_statement.rs +++ b/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()); diff --git a/vhdl_lang/src/syntax/sequential_statement.rs b/vhdl_lang/src/syntax/sequential_statement.rs index c4a8dd2b..bcb76ab6 100644 --- a/vhdl_lang/src/syntax/sequential_statement.rs +++ b/vhdl_lang/src/syntax/sequential_statement.rs @@ -530,8 +530,8 @@ fn parse_selected_assignment(ctx: &mut ParsingContext<'_>) -> ParseResult