possible bug (?)

[Matej Kosik]

Hello,

I have stumbled upon a peculiar problem with the bitstring library.
For your convenience, I have dissected the smallest code that
reproduces it.
The following code:

let rec loop () =
let bits = Bitstring.create_bitstring 1 in
(bitmatch bits with
| { _ } -> loop ()
)
in
loop ();

should loop forever because "loop" is a tail-recursive function.
When I compile and run this program, I get a run-time error:

Fatal error: exception Stack_overflow

I can rewrite the code to work-around this problem, but I still think
that even the original version should work, shouldn't it?
Is this something that should be filed as a bug?

[Matthieu Dubuget]

Le 03/01/2012 12:40, Matej Kosik a �crit :

> Hello,
>
> I have stumbled upon a peculiar problem with the bitstring library.
> For your convenience, I have dissected the smallest code that
> reproduces it.
> The following code:
>
> let rec loop () =
> let bits = Bitstring.create_bitstring 1 in
> (bitmatch bits with
> | { _ } -> loop ()
> )
> in
> loop ();
>
> should loop forever because "loop" is a tail-recursive function.

Are you sure of that?

bitmatch is expanded to something more complex through the syntax extension.

Salutations

Matt

[Matej Kosik]

On 01/03/2012 11:55 AM, Matthieu Dubuget wrote:

> Le 03/01/2012 12:40, Matej Kosik a écrit :
>> Hello,
>>
>> I have stumbled upon a peculiar problem with the bitstring library.
>> For your convenience, I have dissected the smallest code that
>> reproduces it.
>> The following code:
>>
>> let rec loop () =
>> let bits = Bitstring.create_bitstring 1 in
>> (bitmatch bits with
>> | { _ } -> loop ()
>> )
>> in
>> loop ();
>>
>> should loop forever because "loop" is a tail-recursive function.
>
> Are you sure of that?
>
> bitmatch is expanded to something more complex through the syntax
> extension.

If, in this case, `bitmatch' construct expands to something that breaks
tail-recursiveness of `loop' function, that surprised me. "Bug" is
probably too strong a word.

I am not sure what others think about it.
Cannot `bitmatch' expansion be changed to preserve tail-recursiveness of
`loop'. The positive result would be that its behavior will not be
surprising. I am not sure if that is realistic. I am not well acquainted
with camlp4.
--
Matej Košík

[Matthieu Dubuget]

Le 03/01/2012 13:35, Matej Kosik a écrit :
> On 01/03/2012 11:55 AM, Matthieu Dubuget wrote:
>> Le 03/01/2012 12:40, Matej Kosik a écrit :
>>> Hello,
>>>
>>> I have stumbled upon a peculiar problem with the bitstring library.
>>> For your convenience, I have dissected the smallest code that
>>> reproduces it.
>>> The following code:
>>>
>>> let rec loop () =
>>> let bits = Bitstring.create_bitstring 1 in
>>> (bitmatch bits with
>>> | { _ } -> loop ()
>>> )
>>> in
>>> loop ();
>>>
>>> should loop forever because "loop" is a tail-recursive function.
>>
>> Are you sure of that?
>>
>> bitmatch is expanded to something more complex through the syntax
>> extension.
>
> If, in this case, `bitmatch' construct expands to something that
> breaks tail-recursiveness of `loop' function, that surprised me. "Bug"
> is probably too strong a word.

The first step is to check. I did not.

Salutations

Matt

[Richard W.M. Jones]

On Tue, Jan 03, 2012 at 12:35:08PM +0000, Matej Kosik wrote:
> On 01/03/2012 11:55 AM, Matthieu Dubuget wrote:

> >Le 03/01/2012 12:40, Matej Kosik a �crit :

As Matthieu correctly determined, bitmatch isn't a straightforward
expansion, even for simple cases.

You can find out what a particular program expands to by using the
camlp4 printer. Take a look at the source to bitstring, file
'Makefile.in', the two rules called 'print-tests' and
'print-examples'.

In the case of your loop above, it should be pretty clear why the
expanded code isn't tail recursive:

let rec loop () =
let bits = Bitstring.create_bitstring 1 in

let (__pabitstring_data_1001, __pabitstring_original_off_1004,
__pabitstring_original_len_1005) =
bits in
let __pabitstring_off_1002 = __pabitstring_original_off_1004
and __pabitstring_len_1003 = __pabitstring_original_len_1005 in
let __pabitstring_off_aligned_1006 = (__pabitstring_off_1002 land 7) = 0
in
(ignore __pabitstring_off_aligned_1006;
let __pabitstring_result_1007 = ref None
in
((try (__pabitstring_result_1007 := Some (loop ()); raise Exit)
with | Exit -> ());
match !__pabitstring_result_1007 with
| Some x -> x
| None -> raise (Match_failure ("test.ml", 3, 3))))
in loop ()

(* camlp4o -I /usr/lib64/ocaml/bitstring bitstring.cma \
bitstring_persistent.cma pa_bitstring.cmo -printer pr_o.cmo test.ml *)

Rich.

--
Richard Jones
Red Hat