[m-rev.] for review: mercury implementation of string__format

Fergus Henderson fjh at cs.mu.OZ.AU
Mon Nov 25 12:13:59 AEDT 2002


On 22-Nov-2002, Peter Ross <pro at missioncriticalit.com> wrote:
> > > runtime/mercury_conf.h.in:
> > > 	#define for existence of ieeefp.h.
> > 
> > It would be nice to also check for isnan() and isinf() in <math.h>, which
> > is where C99 puts them.
>
> Done.

In fact, why is this code using ieeefp.h at all?
Which system has isnan() in <ieeefp.h> but not in <math.h>?

> --- aclocal.m4	19 Nov 2002 08:16:05 -0000	1.15
> +++ aclocal.m4	22 Nov 2002 10:25:54 -0000
> +AC_DEFUN(MERCURY_CHECK_FOR_IEEE_FUNC,
> +[
> +AC_REQUIRE([MERCURY_CHECK_FOR_IEEEFP_H])
> +AC_MSG_CHECKING(for $1 function)
> +mercury_cv_ieee_func_define="MR_HAVE_`echo $1 | \
> +	tr abcdefghijklmnopqrstuvwxyz./ ABCDEFGHIJKLMNOPQRSTUVWXYZ__`"
> +
> +AC_TRY_LINK([
> +	#include <math.h>
> +#ifdef MR_HAVE_IEEEFP_H
> +	#include <ieeefp.h>
> +#endif
> +],[
> +	float f;
> +	$1(f);
> +],[mercury_cv_have_ieee_func=yes],[mercury_cv_have_ieee_func=no])

The `float f' here should be declared at the top level
(i.e.  in the first argument to AC_TRY_LINK, not the second);
otherwise this code is accessing an uninitialized variable,
which might cause warnings or even errors from the C compiler
either of which might cause the configure test to fail.

> +++ NEWS	22 Nov 2002 10:25:54 -0000
> @@ -231,6 +231,10 @@
>    float.m and extras/complex_numbers.  (Because of rounding errors,
>    the functions aren't actually reversible.)
>  
> +* We've added the three predicates, `is_nan/1', `is_inf/1' and
> +  `is_nan_or_inf/1' to float.m.  These predicates are for use only on
> +  systems which support IEEE floating point arithmetic.

s/predicates,/predicates/

string.m:
> +	%
> +	% Unlike the standard library function, this function converts a float
> +	% to a string without resorting to scientific notation.
> +	%
> +	% This predicate relies on the fact that string__float_to_string
> +	% returns a float which is round-trippable, ie to the full precision
> +	% needed.
> +	%
> +:- func convert_float_to_string(float) = string.
> +convert_float_to_string(Float) = String :-
> +	FloatStr = string__float_to_string(Float),

Should that use string__lowlevel_float_to_string?

This change adds a lot of new code to string.m, which in the usual
case of compiling to C (i.e. using_sprintf succeeds) will never get used.
It would be nicer to put the new code in a new (private) module,
so that it gets compiled to a separate object file and hence doesn't
get linked in if there are no references to it.  (Hopefully the C compiler
should be able to spot the fact that using_sprintf always succeeds,
and hence be able to delete the code for the not using_sprintf case --
at least for the high-level C grades.)

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list