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

Julien Fischer jfischer at opturion.com
Sun Feb 15 16:01:15 AEDT 2015



Hi Peter,

Given that this change adds new test cases, you should probably coordinate
when you commit it with Zoltan, since he is restructuring the test suite
at the moment.

On Fri, 13 Feb 2015, Peter Wang wrote:

> Make base_string_to_int check for overflow and underflow when converting
> from strings in all bases, not only base 10.
>
> Previously it was stated that "numbers not in base 10 are assumed to
> denote bit patterns and are not checked for overflow."  Though not a
> safe assumption in general, in Mercury source files it is useful to be
> able to write values with the high bit set, e.g. 0x80000000 on 32-bit
> machines, that would be greater than max_int if interpreted as a
> positive integer.
>
> The changed behaviour of base_string_to_int would reject such literals
> from Mercury sources, so additional changes are required to maintain
> that usage.  However, unlike before, the compiler will report an
> error if some non-zero bits of the literal would be discarded.
>
> library/string.m:
> 	Enable overflow/underflow checking for base_string_to_int for
> 	any base.
>
> 	Update documentation.
>
> library/lexer.m:
> 	Allow `big_integer' token functor to represent non-base 10
> 	literals as well.
>
> library/term.m:
> 	Add `big_integer' term functor.

Adding big_integer in term.m will potentially break existing code
that uses that module.  Please add something to the NEWS file informing
users about this aspect change.

> 	Pass through `big_integer' tokens as `big_integer' terms.
>
> 	Conform to changes.
>
> compiler/prog_util.m:
> 	Add predicate to convert integer literals in Mercury sources to
> 	ints, with the aforementioned concession for bit patterns.
>
> 	`make_functor_cons_id' can now fail due to integer tokens
> 	exceeding the range of `int'.
>
> compiler/superhomogeneous.m:
> 	Make `unravel_var_functor_unification' convert `big_integer'
> 	tokens on the RHS to a simple `int' with the aforementioned
> 	concession for bit patterns, or add an error message if any
> 	significant bits would be discarded.
>
> compiler/fact_table.m:
> compiler/mercury_to_mercury.m:
> compiler/module_imports.m:
> compiler/prog_io_util.m:
> 	Conform to changes.
>
> compiler/make.util.m:
> 	Delete unused predicate.
>
> tests/general/test_string_to_int_overflow.m:
> tests/general/test_string_to_int_overflow.exp:
> tests/general/test_string_to_int_overflow.exp2:
> tests/general/test_string_to_int_overflow.exp3:
>        Rewrite test case.
>
> tests/hard_coded/lexer_bigint.exp:
> tests/hard_coded/lexer_bigint.exp2:
> tests/hard_coded/read_min_int.exp:
> tests/hard_coded/read_min_int.exp2:
> 	Update expected outputs due to the lexer and term module changes.
>
> tests/invalid/Mmakefile:
> tests/invalid/invalid_int.err_exp:
> tests/invalid/invalid_int.err_exp2:
> tests/invalid/invalid_int.m:
> 	Add new test case.
>
> NEWS:
> 	Announce the changes.
>
> diff --git a/NEWS b/NEWS
> index bfa7323..9a8a016 100644

...


> 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.

...

> diff --git a/tests/invalid/invalid_int.err_exp2 b/tests/invalid/invalid_int.err_exp2
> new file mode 100644
> index 0000000..4bf4dc0
> --- /dev/null
> +++ b/tests/invalid/invalid_int.err_exp2
> @@ -0,0 +1,6 @@
> +invalid_int.m:021: Error: integer literal is too big
> +invalid_int.m:021:   `0b10000000000000000000000000000000000000000000000000000000000000000'.
> +invalid_int.m:026: Error: integer literal is too big
> +invalid_int.m:026:   `0o2000000000000000000000'.
> +invalid_int.m:032: Error: integer literal is too big `0x10000000000000000'.
> +invalid_int.m:037: Error: integer literal is too big `9223372036854775808'.

There seem to be some rubbish characters in the expected error there.

The change looks fine otherwise.

Cheers,
Julien.



More information about the reviews mailing list