AdaCore / svd2ada

An Ada binding generator from SVD descriptions for bare board ARM devices.
GNU General Public License v3.0
64 stars 36 forks source link

Unnecessary 'use' clause #59

Closed simonjwright closed 4 years ago

simonjwright commented 5 years ago

I run svd2ada with e.g.

$(SVD2ADA)/svd2ada          \
--output=$@             \
--package=STM32F40x         \
--no-vfa-on-types           \
$(SVD2ADA)/CMSIS-SVD/ST/STM32F40x.svd

and compile with switches including -gnatwe and get

stm32f40x.ads:7:19: warning: use clause for package "Interfaces" has no effect
gprbuild: *** compilation phase failed

The offending use clause can be removed without any trouble (so far as I can tell; I did try other options, no problems, but no other MCUs).

I did work out a patch:

diff --git a/src/ada_gen.adb b/src/ada_gen.adb
index ac091db..d23be04 100644
--- a/src/ada_gen.adb
+++ b/src/ada_gen.adb
@@ -1756,7 +1756,7 @@ package body Ada_Gen is
    is
    begin
       if Element.Size in 8 | 16 | 32 | 64 then
-         Add (Spec, New_With_Clause ("Interfaces", True));
+         Add (Spec, New_With_Clause ("Interfaces"));
       end if;
    end Added_In_Spec;

but since that call is the only one in which New_With_Clause.Use_Visible is passed as True perhaps Use_Visible should be eliminated altogether?

pat-rogers commented 5 years ago

Hi Simon,

I'm assuming that the clause for Interfaces is required by child packages, but have not checked.

simonjwright commented 5 years ago

Pat,

The complete STM32F4 set will build happily without the use clause, and so would NRF51 were it not for two other problems (and even those are because of my pesky use of -gnatwe).

Since the uses of Interfaces are (for STM32F4, anyway) only to declare types in the top-level package, e.g. type UInt32 is new Interfaces.Unsigned_32;, rather than subtypes, wouldn’t any operations be OK without the use clause?

pat-rogers commented 5 years ago

Yes, that specific type would have the operations visible, but I was thinking perhaps child packages referenced the content of package Interfaces directly. But if not then I agree we can remove it.

simonjwright commented 4 years ago

Fixed in #64