On Fri, Dec 28, 2012 at 03:27:36PM +0100, Felix wrote: > >> Attached is a patch that corrects the behaviour of > >> C_i_foreign_[unsigned]_integer64_argumentp (there where bugs in both > >> the signed and unsigned variant). > > > > Thanks! A question though: Shouldn't C_i_foreign_integer64_argumentp > > also check that the flonum is an integer (ie, there's nothing after the > > decimal point)? > > It could. Should it? C just truncates. I dunno.
I think it should. This will be an extra help in detecting bugs; if a user accidentally passes a flonum where a fixnum is expected that's always a bug, unless the flonum is the result of a fixnum overflow. And in the latter case, this should always be an integer value. To catch accidental misuse the additional check is useful. I've attached a modified version of your patch, which I signed off. > > I think we should seriously consider supporting bignums in core. > > Argh. I know :) > > They won't add much extra overhead in terms of optimizations > > I think quite the opposite is the case, but I'm usually wrong with my > performance estimations. The only option is probably to try it out, > which is a lot of hard work. Yep. > > [the added exactness precision] at least > > allows for some (admittedly trivial) optimizations you don't get when > > using flonums, like knowing that integer? and exact? return true. > > I don't think that it adds many optimization options. I agree. > > Besides, you won't lose information which helps srfi-4 64 bit types make > > sense, and simplifies the part of the FFI currently under consideration. > > That is one case. It also _could_ simplify a lot of FFI code and handling > of word-sized integers in general. I hope so. > I get cramps thinking of all the work that would have to go into such > a project. Yeah, me too. But it's good to hear you aren't against it per se. I might give this a try sometime. There are some improvements I want to make in the numbers egg first, which then should make integration into core simpler. I know the numbers egg improvements have been slow going, but it's a fine goal for the new year ;) Cheers, Peter -- http://sjamaan.ath.cx
>From 5d4c4b5b70e65af4be5c5d53a9f7e7c7f764326c Mon Sep 17 00:00:00 2001 From: felix <[email protected]> Date: Fri, 28 Dec 2012 00:13:27 +0100 Subject: [PATCH] Corrected behaviour for "C_i_foreign_[unsigned]_integer64_argumentp" Extract floating-point values from argument and compare with MIN/MAX for the associated C type). Added limits to chicken.h, which uses stdint.h now (or inttypes.h on SunOS). Disabled compiler-test added by Peter for #955 for 32-bit platforms. Signed-off-by: Peter Bex <[email protected]> --- chicken.h | 55 +++++++++++++++++++++++++++++++++++++++++++------------ runtime.c | 19 ++++++++++++------- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/chicken.h b/chicken.h index e128332..082c318 100644 --- a/chicken.h +++ b/chicken.h @@ -502,9 +502,17 @@ static inline int isinf_ld (long double x) #define C_F32_LOCATIVE 8 #define C_F64_LOCATIVE 9 +#if defined (__MINGW32__) +# define C_s64 __int64 +# define C_u64 unsigned __int64 +#else +# define C_s64 int64_t +# define C_u64 uint64_t +#endif + #ifdef C_SIXTY_FOUR # ifdef C_LLP -# define C_word __int64 +# define C_word C_s64 # else # define C_word long # endif @@ -516,24 +524,47 @@ static inline int isinf_ld (long double x) # define C_s32 int #endif -#if defined (__MINGW32__) -# define C_s64 __int64 -# define C_u64 unsigned __int64 -#else -# define C_s64 int64_t -# define C_u64 uint64_t -#endif - #define C_char char #define C_uchar unsigned C_char #define C_byte char #define C_uword unsigned C_word #define C_header C_uword +#if defined(__sun__) && !defined(__svr4__) +/* SunOS is supposed not to have stdint.h */ +# include <inttypes.h> +#else +# include <stdint.h> +#endif + +/* if all else fails, use these: + #define UINT64_MAX (18446744073709551615ULL) + #define INT64_MAX (9223372036854775807LL) + #define INT64_MIN (-INT64_MAX - 1) + #define UINT32_MAX (4294967295U) + #define INT32_MAX (2147483647) + #define INT32_MIN (-INT32_MAX - 1) + #define UINT16_MAX (65535U) + #define INT16_MAX (32767) + #define INT16_MIN (-INT16_MAX - 1) + #define UINT8_MAX (255) + #define INT8_MAX (127) + #define INT8_MIN (-INT8_MAX - 1) +*/ + +#define C_U64_MAX UINT64_MAX +#define C_S64_MIN INT64_MIN +#define C_S64_MAX INT64_MAX + #if defined(C_LLP) && defined(C_SIXTY_FOUR) -# define C_long __int64 -# define C_LONG_MAX LONG_LONG_MAX -# define C_LONG_MIN LONG_LONG_MIN +# define C_long C_s64 +# ifndef LONG_LONG_MAX +# define C_LONG_MAX LLONG_MAX +# define C_LONG_MIN LLONG_MIN +# else +# define C_LONG_MAX LONG_LONG_MAX +# define C_LONG_MIN LONG_LONG_MIN +# endif #else # define C_long long # define C_LONG_MAX LONG_MAX diff --git a/runtime.c b/runtime.c index 63e553c..0b89361 100644 --- a/runtime.c +++ b/runtime.c @@ -5879,10 +5879,15 @@ C_regparm C_word C_fcall C_i_foreign_integer_argumentp(C_word x) C_regparm C_word C_fcall C_i_foreign_integer64_argumentp(C_word x) { - double m; + double m, r; - if((x & C_FIXNUM_BIT) != 0 || (!C_immediatep(x) && C_block_header(x) == C_FLONUM_TAG)) - return x; + if((x & C_FIXNUM_BIT) != 0) return x; + + if(!C_immediatep(x) && C_block_header(x) == C_FLONUM_TAG) { + m = C_flonum_magnitude(x); + + if(m >= C_S64_MIN && m <= C_S64_MAX && C_modf(m, &r) == 0.0) return x; + } barf(C_BAD_ARGUMENT_TYPE_NO_INTEGER_ERROR, NULL, x); return C_SCHEME_UNDEFINED; @@ -5891,14 +5896,14 @@ C_regparm C_word C_fcall C_i_foreign_integer64_argumentp(C_word x) C_regparm C_word C_fcall C_i_foreign_unsigned_integer_argumentp(C_word x) { - double m; + double m ,r; if((x & C_FIXNUM_BIT) != 0) return x; if(!C_immediatep(x) && C_block_header(x) == C_FLONUM_TAG) { m = C_flonum_magnitude(x); - if(m >= 0 && m <= C_UWORD_MAX) return x; + if(m >= 0 && m <= C_UWORD_MAX && C_modf(m, &r) == 0.0) return x; } barf(C_BAD_ARGUMENT_TYPE_NO_UINTEGER_ERROR, NULL, x); @@ -5908,14 +5913,14 @@ C_regparm C_word C_fcall C_i_foreign_unsigned_integer_argumentp(C_word x) C_regparm C_word C_fcall C_i_foreign_unsigned_integer64_argumentp(C_word x) { - double m; + double m, r; if((x & C_FIXNUM_BIT) != 0) return x; if(!C_immediatep(x) && C_block_header(x) == C_FLONUM_TAG) { m = C_flonum_magnitude(x); - if(m >= 0 && m <= C_UWORD_MAX) return x; + if(m >= 0 && m <= C_U64_MAX && C_modf(m, &r) == 0.0) return x; } barf(C_BAD_ARGUMENT_TYPE_NO_UINTEGER_ERROR, NULL, x); -- 1.8.0.1
_______________________________________________ Chicken-hackers mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/chicken-hackers
