Newbie's question

Newbie's question

[Christos Chryssochoidis]

Hello,

I 'm experimenting with Ada, and I'm having trouble getting some toy 
code running. The code is the following: (hope it isn't too complicated)


-- This is a program to test passing subprograms as arguments to other 
subprograms.
-- It consists of a main program, "Test_Filter", and two nested 
subprograms: "Filter" and "Greater_Equal_3". The "Filter" subprogram 
takes as arguments an array of Integers and a function (predicate), and 
returns an array of the same type, having only those elements of the 
input array that satisfy the given predicate. For some reason that I 
can't figure out, I'm getting a "segmentation fault" when I run the 
executable. (It compiles fine.)


with Ada.Containers.Doubly_Linked_Lists;
with Ada.Text_IO;

procedure Test_Filter is

    subtype Index is Natural;
    type Int_Array is array(Index range<>) of Integer;


    function Filter(Elements : Int_Array; Predicate : not null 
access 					function(Element : Integer) return Boolean) 								return 
Int_Array  is
       package Int_Lists is new 									 
Ada.Containers.Doubly_Linked_Lists(Element_Type => Integer);
       Tmp_List : Int_Lists.List;
       Result : access Int_Array;
    begin
       for I in Elements'Range loop
          if Predicate(Elements(I)) then
             Tmp_List.Append(Elements(I));
          end if;
       end loop;

       Result := new Int_Array(1..Index(Tmp_List.Length));
   Copy_List:
       declare
          Tmp_List_Cursor : Int_Lists.Cursor := Tmp_List.First;
          I : Integer := 1;
       begin
          while Int_Lists.Has_Element(Tmp_List_Cursor) loop
             Result(I) := Int_Lists.Element(Tmp_List_Cursor);
             Tmp_List_Cursor := Int_Lists.Next(Tmp_List_Cursor);
             I := I + 1;
          end loop;
       end Copy_List;

       return Result.all;

    end Filter;

    function Greater_Equal_3(Element :Integer) return Boolean is
    begin
       if Element >= 3 then
          return True;
       else
          return False;
       end if;
    end Greater_Equal_3;


    Int_Array1 : Int_Array := Int_Array'(1,2,3,4,5);
    Int_Array2 : Int_Array(1..3);

begin
    Int_Array2 := Filter(Int_Array1, Greater_Equal_3'Access);
    for I in Int_Array1'Range loop
       Ada.Text_IO.Put_Line(Int_Array1(I)'Img);
    end loop;

    Ada.Text_IO.Put_Line("");

    for I in Int_Array2'Range loop
       Ada.Text_IO.Put_Line(Int_Array2(I)'Img);
    end loop;

end Test_Filter;


Thanks very much for any help.

Re: Newbie's question [SOLVED]

[Christos Chryssochoidis]

I changed version of GNAT and now the code runs ok.



Christos Chryssochoidis wrote:
> 
> Hello,
> 
> I 'm experimenting with Ada, and I'm having trouble getting some toy 
> code running. The code is the following: (hope it isn't too complicated)
> 
> 
> -- This is a program to test passing subprograms as arguments to other 
> subprograms.
> -- It consists of a main program, "Test_Filter", and two nested 
> subprograms: "Filter" and "Greater_Equal_3". The "Filter" subprogram 
> takes as arguments an array of Integers and a function (predicate), and 
> returns an array of the same type, having only those elements of the 
> input array that satisfy the given predicate. For some reason that I 
> can't figure out, I'm getting a "segmentation fault" when I run the 
> executable. (It compiles fine.)
> 
> 
> with Ada.Containers.Doubly_Linked_Lists;
> with Ada.Text_IO;
> 
> procedure Test_Filter is
> 
>    subtype Index is Natural;
>    type Int_Array is array(Index range<>) of Integer;
> 
> 
>    function Filter(Elements : Int_Array; Predicate : not null 
> access                     function(Element : Integer) return 
> Boolean)                                 return Int_Array  is
>       package Int_Lists is new                                      
> Ada.Containers.Doubly_Linked_Lists(Element_Type => Integer);
>       Tmp_List : Int_Lists.List;
>       Result : access Int_Array;
>    begin
>       for I in Elements'Range loop
>          if Predicate(Elements(I)) then
>             Tmp_List.Append(Elements(I));
>          end if;
>       end loop;
> 
>       Result := new Int_Array(1..Index(Tmp_List.Length));
>   Copy_List:
>       declare
>          Tmp_List_Cursor : Int_Lists.Cursor := Tmp_List.First;
>          I : Integer := 1;
>       begin
>          while Int_Lists.Has_Element(Tmp_List_Cursor) loop
>             Result(I) := Int_Lists.Element(Tmp_List_Cursor);
>             Tmp_List_Cursor := Int_Lists.Next(Tmp_List_Cursor);
>             I := I + 1;
>          end loop;
>       end Copy_List;
> 
>       return Result.all;
> 
>    end Filter;
> 
>    function Greater_Equal_3(Element :Integer) return Boolean is
>    begin
>       if Element >= 3 then
>          return True;
>       else
>          return False;
>       end if;
>    end Greater_Equal_3;
> 
> 
>    Int_Array1 : Int_Array := Int_Array'(1,2,3,4,5);
>    Int_Array2 : Int_Array(1..3);
> 
> begin
>    Int_Array2 := Filter(Int_Array1, Greater_Equal_3'Access);
>    for I in Int_Array1'Range loop
>       Ada.Text_IO.Put_Line(Int_Array1(I)'Img);
>    end loop;
> 
>    Ada.Text_IO.Put_Line("");
> 
>    for I in Int_Array2'Range loop
>       Ada.Text_IO.Put_Line(Int_Array2(I)'Img);
>    end loop;
> 
> end Test_Filter;
> 
> 
> Thanks very much for any help.

Re: Newbie's question

[Jeffrey R. Carter]

Christos Chryssochoidis wrote:

Apparently you found a compiler error. Some comments on your code:

>       Result : access Int_Array;
>    begin
>       for I in Elements'Range loop
>          if Predicate(Elements(I)) then
>             Tmp_List.Append(Elements(I));
>          end if;
>       end loop;
> 
>       Result := new Int_Array(1..Index(Tmp_List.Length));

This is OK for a little test program like this, but in real code, you have a 
memory leak from this use of access values and heap allocation that you should 
want to avoid. You can do that by moving the declaration of Result into 
Copy_List below:

Result : Int_Array (1 .. Index (Tmp_List.Length) );

You might also find an unbounded array (Ada.Containers.Vectors) more suited than 
a list here, since you can use the same index for both.

>   Copy_List:
>       declare
>          Tmp_List_Cursor : Int_Lists.Cursor := Tmp_List.First;
>          I : Integer := 1;
>       begin
>          while Int_Lists.Has_Element(Tmp_List_Cursor) loop
>             Result(I) := Int_Lists.Element(Tmp_List_Cursor);
>             Tmp_List_Cursor := Int_Lists.Next(Tmp_List_Cursor);
>             I := I + 1;
>          end loop;
>       end Copy_List;

Rather than using a cursor and a loop, I would probably use the Iterate 
procedure to convert the list into an array.

You could also make Filter recursive and eliminate Result and Tmp_List altogether:

begin -- Filter
    if Elements'Length <= 0 then
       return Elements;
    elsif Predicate (Elements (Elements'First) ) then
       return Elements'First &
              Filter (Elements (Elements'First + 1 .. Elements'Last), Predicate);
    else
       return Filter (Elements (Elements'First + 1 .. Elements'Last), Predicate);
    end if;
end Filter;

>    function Greater_Equal_3(Element :Integer) return Boolean is
>    begin
>       if Element >= 3 then
>          return True;
>       else
>          return False;
>       end if;
>    end Greater_Equal_3;

This should be

return Element >= 3;

Personally, I'd make filter generic rather than passing the Predicate function 
as a subprogram parameter.

-- 
Jeff Carter
"Spam! Spam! Spam! Spam! Spam! Spam! Spam! Spam!"
Monty Python's Flying Circus
53

Re: Newbie's question

[Christos Chryssochoidis]

Jeffrey R. Carter wrote:
> 
> Christos Chryssochoidis wrote:
> 
> Apparently you found a compiler error. Some comments on your code:
> 
>>       Result : access Int_Array;
>>    begin
>>       for I in Elements'Range loop
>>          if Predicate(Elements(I)) then
>>             Tmp_List.Append(Elements(I));
>>          end if;
>>       end loop;
>>
>>       Result := new Int_Array(1..Index(Tmp_List.Length));
> 
> This is OK for a little test program like this, but in real code, you 
> have a memory leak from this use of access values and heap allocation 
> that you should want to avoid. You can do that by moving the declaration 
> of Result into Copy_List below:
> 
> Result : Int_Array (1 .. Index (Tmp_List.Length) );

I ruled out the declaration of a local array within the block statement 
Copy_List, because I thought that the compiler would mandate a return 
statement to be at the top level of the function's body, which I 
wouldn't be able to do, since the declared array would be local to the 
block statement. Surprisingly I realized I was wrong; this solution 
compiles fine. The compiler doesn't complain about not finding a return 
statement at the outermost scope of the function.



> 
> You might also find an unbounded array (Ada.Containers.Vectors) more 
> suited than a list here, since you can use the same index for both.

Good point.

> 
>>   Copy_List:
>>       declare
>>          Tmp_List_Cursor : Int_Lists.Cursor := Tmp_List.First;
>>          I : Integer := 1;
>>       begin
>>          while Int_Lists.Has_Element(Tmp_List_Cursor) loop
>>             Result(I) := Int_Lists.Element(Tmp_List_Cursor);
>>             Tmp_List_Cursor := Int_Lists.Next(Tmp_List_Cursor);
>>             I := I + 1;
>>          end loop;
>>       end Copy_List;
> 
> Rather than using a cursor and a loop, I would probably use the Iterate 
> procedure to convert the list into an array.

I tried it. (Code below.) It works fine. Thanks.


Copy_List:
       declare
          Result : Int_Array(0..Index(Tmp_Vector.Length)-1);
          I : Integer := 0;
          procedure Copy(Position: Int_Vectors.Cursor) is
          begin
             Result(I) := Int_Vectors.Element(Position);
             I := I + 1;
          end Copy;

       begin
          Tmp_Vector.Iterate(Copy'Access);

          return Result;
       end Copy_List;





> 
> You could also make Filter recursive and eliminate Result and Tmp_List 
> altogether:
> 
> begin -- Filter
>    if Elements'Length <= 0 then
>       return Elements;
>    elsif Predicate (Elements (Elements'First) ) then
>       return Elements'First &
>              Filter (Elements (Elements'First + 1 .. Elements'Last), 
> Predicate);
>    else
>       return Filter (Elements (Elements'First + 1 .. Elements'Last), 
> Predicate);
>    end if;
> end Filter;

I thought too about doing it recursively, but stubbornly I wanted to 
find a way to do that iteratively, as that's how I would do it in other 
languages I 'm familiar with…

> 
>>    function Greater_Equal_3(Element :Integer) return Boolean is
>>    begin
>>       if Element >= 3 then
>>          return True;
>>       else
>>          return False;
>>       end if;
>>    end Greater_Equal_3;
> 
> This should be
> 
> return Element >= 3;

That's much simpler, I agree.

> 
> Personally, I'd make filter generic rather than passing the Predicate 
> function as a subprogram parameter.
> 

I made another version of my code using a generic subprogram as you 
suggested. That's also an elegant solution.

Thanks very much for all your suggestions.

Re: Newbie's question

[Christos Chryssochoidis]

 From a reply I received in private, it seems that the "generic" version 
of that code of mine is of interest for some, so I post it, making no 
claim that it is well written…







with Ada.Containers.Vectors;
with Ada.Text_IO;

procedure Test_Filter is

    subtype Index is Natural;
    type Int_Array is array(Index range<>) of Integer;


    generic
       with function Predicate(Element : Integer) return Boolean;
    function Filter(Elements : Int_Array) return Int_Array;


    function Filter(Elements : Int_Array) return Int_Array is
       package Int_Vectors is new Ada.Containers.Vectors(Element_Type => 
Integer, Index_Type => Index);
       Tmp_Vector : Int_Vectors.Vector;


    begin
       for I in Elements'Range loop
          if Predicate(Elements(I)) then
             Tmp_Vector.Append(Elements(I));
          end if;
       end loop;

   Copy_Vector:
       declare
          Result : Int_Array(0..Index(Tmp_Vector.Length)-1);
          I : Integer := 0;
          procedure Copy(Position: Int_Vectors.Cursor) is
          begin
             Result(I) := Int_Vectors.Element(Position);
             I := I + 1;
          end Copy;

       begin
          Tmp_Vector.Iterate(Copy'Access);

          return Result;
       end Copy_Vector;



    end Filter;

    function Greater_Equal_3(Element :Integer) return Boolean is
    begin
       return Element <= 3;
    end Greater_Equal_3;


    Int_Array1 : Int_Array := (1,2,3,4,5);
    Int_Array2 : Int_Array(1..3);
    function My_Filter is new Filter(Predicate=>Greater_Equal_3);
begin
    Int_Array2 := My_Filter(Int_Array1);
    for I in Int_Array1'Range loop
       Ada.Text_IO.Put_Line(Int_Array1(I)'Img);
    end loop;

    Ada.Text_IO.Put_Line("");

    for I in Int_Array2'Range loop
       Ada.Text_IO.Put_Line(Int_Array2(I)'Img);
    end loop;

end Test_Filter;