Topic/unsigned

[reznikmm]

In Ada, data types are not used based on their physical properties. Unlike C, where uint8, uint32 are used everywhere a corresponding range of values is required, in Ada, types are created where the type represents semantic value. This is achieved by the fact that Ada has advanced means of specifying the physical properties of components and variables, and these properties do not depend on the type used. You can always force the compiler to allocate, for example, 2 bits for an Integer type by specifying a range constraint and representation specifier for a specific field. Since it is not possible to know which fields of the SVD description correspond to which semantic types, there is no need to create multiple types for the fields. These created types do not carry useful information and only force the user to insert type conversions to stop the compiler from complaining. It would be more logical to use a single unsigned integer type and use it to describe all components in SVD record types.

This patch adds the --use-unsigned-type=<UInt32> command line option. Specifying this option forces svd2ada to use this type (from the package of --base-types-package=) for the declaration of all components of records and arrays. Corresponding range constraints range 0 .. 2**W-1 are added, where W is the number of bits.

The SVD packages created in this way do not depend on HAL, which simplifies splitting into crates.

CC @godunko @ohenley

[CLAassistant]

All committers have signed the CLA.

[Irvise]

I personally think this is a good idea, if it is "hidden" within a flag, which it is. So I would like to see it within SVD2Ada. It gives another option, while not being the default.

[Fabien-Chouteau]

The problem is that there is still a type (even if anonymous) because you restrict the range. And you are introducing implicit conversions to this anonymous type; this is error-prone.

In Ada, data types are not used based on their physical properties. Unlike

Here we want the physical property because that's the only thing SVD is giving us.

[reznikmm]

Consider two launch of svd2ada:

./bin/svd2ada --package=Root --base-types-package=HAL --boolean -o old --gen-uint-always CMSIS-SVD/ST/STM32F40x.svd
# vis
./bin/svd2ada --package=Root --base-types-package=Interfaces --boolean -o new --use-unsigned-type=Unsigned_32 --gen-uint-always CMSIS-SVD/ST/STM32F40x.svd 

Comparing Root.ADC packages we will see:

-   subtype CR1_AWDCH_Field is HAL.UInt5;
+   subtype CR1_AWDCH_Field is Interfaces.Unsigned_32 range 0 .. 31;

If we have usage like

Root.ADC.ADC1_Periph.CR1.AWDCH := 99;

In the first case compiler output will be:

svd_test.adb:5:38: error: value not in range of type "CR1_AWDCH_Field" defined at root-adc.ads:47
svd_test.adb:5:38: error: static expression fails Constraint_Check

In the second case:

svd_test.adb:5:38: warning: value not in range of type "CR1_AWDCH_Field" defined at root-adc.ads:47 [enabled by default]
svd_test.adb:5:38: warning: Constraint_Error will be raised at run time [enabled by default]

So, compiler spots this in both cases.

[kevlar700]

I think I like this. Minimum sizes are still required for use record (component clause). The only perhaps minor issue I have found so far is that with e.g. HAL.UInt7 you can get a warning such as:

component size overrides size clauses for "HAL.UInt7" [-gnatw.s].

When the last range of a records components in the use record (component clause) is too large.

[kevlar700]

It would also seem to provide for ranges that are more accurate such as values 0 .. 64 in 8 bits (where enumerations are not most appropriate) at the cost of lesser clarity in how many bits each register represents. Though this clarity issue could perhaps be overcome by using byte offsets other than 0. Though I think that would be a bit of an implementation pain when a register spans two registers (a non micro device I am currently working on without SVD does this). I'm also not sure if spanning registers partially is SVD compliant anyway.

Register_1 at 0 range 0 .. 7;
Register_2 at 1 range 0 .. 7;
Register_3 at 2 range 0 .. 7;
Register_4 at 3 range 0 .. 6;
Register_5 at 3 range 7 .. 15;
Register_6 at 4 range 0 .. 7;

[kevlar700]

There is a higher chance of a runtime exception though. If a procedures parameter is for some accidental reason Unsigned_32 and 256 gets passed through to an Unsigned_32 with range 0 .. 255. SPARK would probably catch that but Ada would have an exception at runtime.

[Fabien-Chouteau]

In the first case compiler output will be:

The compiler will only detect this for basic cases on assigning with a constant. My point holds, this is just hiding potential bug.