VUnit / vunit

VUnit is a unit testing framework for VHDL/SystemVerilog
http://vunit.github.io/
Other
697 stars 250 forks source link

ModelSim issues warning regarding `string_ops.vhd` #981

Closed nselvara closed 3 months ago

nselvara commented 6 months ago

Hello, when I run VUnit test the ModelSim spits out a warning regarding the string_ops The warning is: \vunit\vhdl\string_ops\src\string_ops.vhd(565): (vopt-1083) Implicit array operator "=" always returns FALSE (left length 1 is not equal to right length 0).

The flagged line of code is this: image

I think the if clause can be rewritten to:

if substring'length = 0 then
    return start_pos;
end if;

Edit

Forgot to mention that it warns at 2 places. image

LarsAsplund commented 4 months ago

Failed to reproduce this myself with my versions of Modelsim and Questasim. Have you verified that the warning goes away with the suggested change? If it thinks the substring has a length of 1 the substring'length = 0 would also always be false.

nselvara commented 4 months ago

@LarsAsplund, Hi, thx for reply. I now checked again and also replaced it with the suggested solution. The warning for real disappears. I tried with both variants. I'd guess 1 is probably the correct one, as the string type itself is at least of length 1.

LarsAsplund commented 4 months ago

The if statement covers a corner case in the find function: What should it return when we search for the empty string? Just like the find method in Python, we return 0 in this case.

The find function comes in two variants: One searching for input parameter substring of type string and one searching for input parameter char of type character. The former can have the empty string as input which motivates the if statement. The latter creates a one-character string of the input char and then calls the former. In that case the input substring to the former will always be of length 1.

  function find (
    constant s : string;
    constant char : character;
    constant start : natural := 0;
    constant stop : natural := 0)
    return natural is
    variable substring : string(1 to 1);
  begin
    substring(1) := char;
    return find(s, substring, start, stop);
  end;

For some reason Questa forgets that the function searching for a string can be called directly. I notice that the string version is declared before the character version but is defined after. Does it make a difference if the definition of the string version is placed first?

nselvara commented 4 months ago

Good morning, I see it seems to be an order issue or some kind of like that. Did you mean like this:

function find (
    constant s : string;
    constant char : character;
    constant start : natural := 0;
    constant stop : natural := 0)
    return natural is
    variable substring : string(1 to 1);
  begin
    substring(1) := char;
    return find(substring, s, start, stop);
  end;

  function find (
    constant substring : string; -- Placing this at the top, right?
    constant s : string;

    constant start : natural := 0;
    constant stop : natural := 0)
    return natural is
    variable start_pos, stop_pos : natural;
    variable o : natural;
  begin
    if start = 0 or left_of_range(s, start) then
      start_pos := s'left;
    elsif right_of_range(s, start) then
      return 0;
    else
      start_pos := start;
    end if;

This doesn't change anything. I still get the warning. It seems to be that the attributes are probably not evaluated in the static analysis of QuestaSim.

I moved the other warning to a new issue https://github.com/VUnit/vunit/issues/993.

LarsAsplund commented 4 months ago

No, I was thinking switching the order of the complete function definitions such that it matches the order the functions were declared. Also, the function depending on the other comes last

nselvara commented 4 months ago

Ahh, now I see, you meant the order of the definition and the declaration. Sry, for misunderstanding it. I give it a try and report back.

Edit

Ok, I tested the two possible variants. First defining and declaring the char version followed by the string variant and the second one was the vice versa. Unfortunately, the simulator still reports the same warnings. 🙁

rafaelnp commented 3 weeks ago

The same issue, the line 488 has the same if statement (string_ops.vhd#L488)branch and got a signal 11 when running questa without the gui on Linux:

# ** Note: (vsim-3812) Design is being optimized...
# ** Warning: /home/user/src/vunit/vunit/vhdl/string_ops/src/string_ops.vhd(488): (vopt-1083) Implicit array operator "=" always returns FALSE (left length 1 is not equal to right length 0).
# vopt_stacktrace.vstf written
# ** Fatal: Unexpected signal: 11.
# ** Note: /**/**/**/**_tb.vhd(203): Vopt Compiler exiting
# ** Error: (vopt-2064) Compiler back-end code generation process terminated with code 232.
# ** Note: (vsim-12126) Error and warning message counts have been restored: Errors=2, Warnings=1.
# Error loading design
Error loading design
# End time: 00:30:41 on Jun 06,2024, Elapsed time: 0:00:01
# Errors: 2, Warnings: 1

When running using questa gui the warning is issued but the simulation runs till the end.

Questa version: image