I like SPARK ;) this is so much better to read than what I would
have expected from a C implementation. Beside the indentation
of course, we need to work on your editor configuration :)

View Change

8 comments:

• File debug/hw-print.adb:

  • Patch Set #1, Line 47: Parse_And_Put

    This is rather Put_And_Parse :)

  • Patch Set #1, Line 82: Put (Format_String (Pos .. Pos));

    Slight optimization: Currently this puts chars one by one. If we'd
    keep the start and end of the non-format string, we could put it all at
    once.

    e.g. on top

       is
          Non_Format_Start : constant Integer := Pos;
          Non_Format_End : Integer;
          ...
       begin

    then here

                      when '%' =>
                              Non_Format_End := Pos;

    and in the end

          if Arg.Conversion = Arg_Invalid then
             Put (Format_String);
          elsif Non_Format_End > Integer'First then
             Put (Format_String (Non_Format_Start .. Non_Format_End - 1));
          end if;

  • Patch Set #1, Line 118: State := State_Arg_Modifier_Length;

    Alternatively, we could skip the `Modifier_Length` substate and use the
    state of `Arg.Length` instead, e.g.

       when State_Arg_Modifier =>
          case Format_String (Pos) is
          when 'l' =>
             case Arg.Length is
             when Arg_Length_Normal =>
                Arg.Length := Arg_Length_Long;
             when Arg_Length_Long =>
                Arg.Length := Arg_Length_Long_Long;
             when others =>
                Pos := Pos - 1;
                State := State_Arg_Conversion;
             end case;
          when others =>
             Pos := Pos - 1;
             State := State_Arg_Conversion;
          end case;

  • Patch Set #1, Line 130: State := State_Arg_Modifier;

    Nit, we already know that `State_Arg_Modifier` will fall through to
    `State_Arg_Conversion`. We can as well go directly to the latter.

  • Patch Set #1, Line 162: type Int_Format is (Base_10, Base_16);

    This enumeration type doesn't seem too useful. Just using

       subtype Int_Format is Positive range 2 .. 16;

    would make the code easier to read (less conversions, but still type safe).

  • Patch Set #1, Line 162: Int_Format

    Maybe `Int_Base` instead?

  • Patch Set #1, Line 166: String_Begin

    Maybe just name it `Pos`. Less clear for the caller, but would make the code in here easier to read.

  • Patch Set #1, Line 207: pragma loop_invariant (Item_C < Item_B ** String_Begin);

    Maybe we have to make it explicit that `Item_B` doesn't change in
    the loop. I'll give that a shot.

To view, visit change 37530. To unsubscribe, or for help writing mail filters, visit settings.