style: data_types

[umarcor]

This PR is based on #488 and it should be merged later.

This PR is work-in-progress towards #476 and #470.

• The style of functions and procedures in almost all the sources in vunit/vhdl/data_types/src is modified:

function
func_name(
  paramA : typeofparamA
) return typeofreturn;

• end function; and end procedure; are replaced with end; for coherency.

• Aliases are added to integer_vector_ptr and string_ptr in order to highlight the similarities between these files. Compare, e.g., integer_vector_ptr_pkg-body-200x.vhd and string_ptr_pkg-body-200x.vhd before and after this PR:

Added(4,28)  Deleted(0,19) Changed(88) Changed in changed(71)
Added(40,16) Deleted(0,25) Changed(63) Changed in changed(43)

• The definition of types integer_vector_t, integer_vector_access_t, string_ptr_access_t, etc. are moved to separate packages in a new subdir (vunit/vhdl/data_types/src/types). This is because, in the future, packages to support external models will depend on these types, and integer_vector_ptr_pkg/string_ptr_pkg will depend on the external models (or their placeholders).

• In the VHDL 2008 sources, there is no need to use custom types for the pointers. 'natural' is used instead.

• When external models are added, the variable inside the pointer types will not be an explicit index. I think that ref is a better name.

• GHDL drops a lot of warnings because declaring parameter length hides the function name. Options are renamed to len to avoid it.

• Parameter index used in get and set is declared as of type integer. However, it should be natural, because it is not possible to access to negative indexes.

[kraigher]

This PR contains some unrelated python changes.

Also the coding style looks weird compared to the other part of the code base. Typically we use a single line for short functions and line break after the function name for long functions.
Have a look at vhdl/com/src/com_api.vhd to get a feel for the coding style. We dont have any code that breaks right after the function/procedure keyword.

[umarcor]

This PR contains some unrelated python changes.

This is because of:

This PR is based on #488 and it should be merged later.

There is a single python change besides those, and it is pertinent: https://github.com/VUnit/vunit/pull/482/files/aa21c26acbb529aa71dcdfef822486a02a8ceb26..738942aa00150c3a83b079061dfd9551a9345581

Also the coding style looks weird compared to the other part of the code base. Typically we use a single line for short functions and line break after the function name for long functions.

I have seen that, and that's why I decided to modify almost all the sources in subdir data_types, and not only the ones that I will modify in upcoming PRs (integer_*, string_*).

The main motivation in favour of the 'new style' is to have 'fixed' locations to make it easier to find subprogram names, arguments/params, the type of the return value, and the type of the params.

Keeping the type of the subprogram in the same line as the name does not allow it, because names of different subprograms are not aligned vertically. Moreover, params after the first one can have multiple possible locations and the vertical aligment is different for each of them. This does not only depend on the number of params, but on the actual names and the length of the type names. The same applies to the return type, which can be located in three different lines, and the vertical aligment is different.

The most disturbing characteristic of the current style for me is that it is hard to know how many params do one-liners have. You need to read the line carefully. This is specially so because : and ; are very similar. For example, it is not easy to spot that these two subprograms are 'complementary':

procedure set(dict : dict_t; key : string := "default"; value : string) is
impure function get(dict : dict_t ; key : string := "default") return string is

procedure
set(
  dict  : dict_t;
  key   : string := "default"
  value : string
);

impure function
get(
  dict : dict_t;
  key  : string := "default"
) return string;

I'm ok with using the following alternative style:

procedure set (
  dict  : dict_t;
  key   : string := "default"
  value : string
);

impure function get (
  dict : dict_t;
  key  : string := "default"
) return string;

Overall, I'd like you to assess the constraint of putting each param in a different line, considering the return type or closing parenthesis as an additional param.

Have a look at vhdl/com/src/com_api.vhd to get a feel for the coding style. We dont have any code that breaks right after the function/procedure keyword.

As commented, this is so that subprogram names are located at a fixed position (indentation) which does not depend on the type of subprogram. In some contexts, procedures and functions can have the same name and arguments: i.e. the same subprogram that returns an exit code or does not. Nonetheless, disregarding this change, the modifications that the 'new style' might imply in vhdl/com/src/com_api.vhd are:

• Move the trailing ); of the procedures to a new line.
• Move the trailing ) return *; of the functions to a new line.
• Convert one-liners to the multi-line format.

[kraigher]

@LarsAsplund what do you think about the coding style proposal here?

[umarcor]

This PR is now independent from any other (#481, #488).

[umarcor]

@kraigher, would it be easier to move forward if I avoided breaks right after the function/procedure keyword?

[LarsAsplund]

@kraigher @umarcor I'm ok with the parameters on separate lines but like to keep the subprogram type on the same line as the name. I think the another approach will cause a lot of tedious linting discussions with people wanting to make contributions. That would be very counterproductive.

Would be nice if we could include such rules in our long checks.

[umarcor]

@kraigher @umarcor I'm ok with the parameters on separate lines but like to keep the subprogram type on the same line as the name.

I updated this PR accordingly. Let we know about any further modifications you might want.

Would be nice if we could include such rules in our long checks.

Do you mean to add some linter? Should it be part of this PR?

[LarsAsplund]

@umarcor My new phone needs to learn about linting... You're right, it would be a separate issue. I have no more comments but I let @kraigher have the last say