Subtypes with `open` don't count as fully-constrained about nvc HOT 4 CLOSED

The function copy_constraints in src/common.c is supposed to copy the constraints from the base type while "filling in the holes" left by open, except it doesn't work properly here. I fixed a similar problem with subtypes of unconstrained records a while ago and I think I was aware this was broken for arrays too but I didn't find time to go back and look at it as that code is quite torturous...

from nvc.

Well, there's also a problem with how this is analyzed in a generic sense. E.g.

   type sample is array (natural range <>) of integer;
   type dozen_short_samples is array(1 to 12) of samples (0 to 9);

is parsed with the 0 to 9 as a constraint on dozen_short_samples instead of creating a new subtype for sample.

from nvc.

Well, there's also a problem with how this is analyzed in a generic sense. E.g.

   type sample is array (natural range <>) of integer;
   type dozen_short_samples is array(1 to 12) of samples (0 to 9);

is parsed with the 0 to 9 as a constraint on dozen_short_samples instead of creating a new subtype for sample.

I don't believe it actually does that: the subtype for samples (0 to 9) is created by p_subtype_indication() at the end of p_constrained_array_type_definition. But p_constrained_array_type_definition also creates a new subtype to hold the (1 to 12) constraint and I think what you're seeing is a side effect of the copy_constraints function which was a flawed attempt to simplify handling of the nested constraints in VHDL-2008 by attaching them to the outermost subtype. Because the above is equivalent to writing:

   type sample is array (natural range <>) of integer;
   type dozen_samples is array (natural range <>) of sample;
   subtype dozen_short_samples is dozen_samples(1 to 12)(0 to 9);

The idea was that copy_constraints would normalise to this form. Anyway it didn't work properly and I've removed it.

from nvc.

Well, that form really should be normalized. Right now there is a 1-to-1 relation between type_ts and c_typeDecls. However, the second example above would require creating a new c_subTypeDecl for the 0 to 9 constraint which doesn't have any corresponding type_t.

Let me provide some motivation for my concern. The matrix_multiply example is something like

library ieee;
use ieee.numeric_std.all;

package matrix_multiplier_pkg is
    type flat_matrix_type is array (integer range <>) of unsigned;
end package;

library ieee ;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use work.matrix_multiplier_pkg.all;

entity matrix_multiplier is
  generic (
    DATA_WIDTH : positive := 8;
    A_ROWS : positive := 8;
    B_COLUMNS : positive := 5;
    A_COLUMNS_B_ROWS : positive := 4
  );
  port (
    clk_i   : in    std_logic;
    reset_i : in    std_logic;
    valid_i : in    std_logic;
    valid_o : out   std_logic;
    a_i     : in    flat_matrix_type(0 to (A_ROWS * A_COLUMNS_B_ROWS) - 1)(DATA_WIDTH - 1 downto 0);
    b_i     : in    flat_matrix_type(0 to (A_COLUMNS_B_ROWS * B_COLUMNS) - 1)(DATA_WIDTH - 1 downto 0);
    c_o     : out   flat_matrix_type(0 to (A_ROWS * B_COLUMNS) - 1)((2 * DATA_WIDTH) + clog2(A_COLUMNS_B_ROWS) - 1 downto 0)
  );
end entity matrix_multiplier;

Note that because the types of these signals are generic, they can't be refactored into subtypes (without removing the generics).

When elaborated, we have a bunch of signals like

signal A_I : in FLAT_MATRIX_TYPE(0 to 31)(7 downto 0);

Where the type of A_I is a subtype of FLAT_MATRIX_TYPE with the constraints 0 to 31 and 7 downto 0. But the element type of FLAT_MATRIX_TYPE us UNSIGNED with no constraints. The type of A_I really should be a subtype of (an unconstrained array of FLAT_MATRIX_TYPE with an element type of (a subtype of UNSIGNED with constraints 7 downto 0)) with constraints 0 to 31.

So later when we try to determine vhpiSizeP for A_I(0) (which should be 8), we can't figure it out because the type of the expression is unconstrained.

from nvc.