[m-rev.] for review: Make base_string_to_int check overflow/underflow for all bases.

Julien Fischer jfischer at opturion.com
Mon Feb 16 13:07:49 AEDT 2015


Hi Peter,

On Mon, 16 Feb 2015, Peter Wang wrote:

> On Sun, 15 Feb 2015 16:01:15 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:

>> On Fri, 13 Feb 2015, Peter Wang wrote:
>>
>>> diff --git a/library/lexer.m b/library/lexer.m
>>> index b7c2a28..cbf755b 100644
>>> --- a/library/lexer.m
>>> +++ b/library/lexer.m
>>> @@ -30,7 +30,10 @@
>>>     --->    name(string)
>>>     ;       variable(string)
>>>     ;       integer(int)
>>> -    ;       big_integer(string) % does not fit in int
>>> +    ;       big_integer(int, string)
>>> +                                % An integer that is too big for `int'. The
>>> +                                % arguments are the base (2, 8, 10, 16) and
>>> +                                % digits of the literal.
>>
>> Is there any good reason for why the base argument is an int and not a enum
>> whose functors correspond to the valid bases, e.g.
>>
>>       :- type big_integer_base
>>          --->    binary
>>          ;       octal
>>          ;       decimal
>>          ;       hexadecimal.
>>
>>
>> Ditto for the version in term.m.
>>
>
> No good reason.  I was a bit worried about duplicating the type
> in lexer and term.
>
> I also considered using `integer' in place of the string representation.
> What do you think?

On balance, I don't have any objection to doing that.  'integer' is
certainly a safer representation than 'string'. 
Also, presuambly if you are going to use the value of a big_integer and
not just spit out an error message, then you will be converting it to an
integer at some point anyway.

The only downside seesm to be that some code may break because it now
needs to import the integer module, but that's easily fixed.

Would you omit the base argument if 'integer' were used?

Cheers,
Julien.



More information about the reviews mailing list