Fix real->flonum in cptypes

[gus-massa]

The expansion used #3%$real->flonum that is marked as discard in primdata.ss, so it was incorrectly deleted in contexts where the result was ignored. See example in cptypes.ms

The new expansion use #2%$real->flonum, so I added a handler for "#2%$real->flonum" in cpprim.ss that calls library.ss. But library.ss had a call #2%$real->flonum that produces a cycle. So I added a new primitive $real->flonum* that is like $real->flonum but it has no handler in cpprim.ss

I tried a few different approaches, but I couldn't make it work otherwise. Anyway, I don't like to use yet another primitive.

Some ideas:

Option 1: Keep the code in the PR as is. It works.

Option 2: Rename #2%$real->flonum* to #2%$real->flonum and use #3%$app/no-inline in library.ss (I tried this approach and failed, perhaps there is a similar method that works.)

Option 3: Simplify $real->flonum* in 5_3.ss so it just calls float instead of checking the types first. This auxiliary function should be called only when all the other handlers gave up, because the argument is a bignum or ratnum. IIRC float had strange results for not numeric arguments, perhaps the result was infinite or something, I don't remember, but it's not a problem.

Option 4: Any better idea is welllcome.

[mflatt]

I like that this cleans up the overwritten $real->flonum in "prims.ss". Before this change, (#%$real->flonum 7 7) produces 7.0, because the "5_3.ss" variant took precedence and didn't check who.

It does seem annoying to introduce yet another layer as a new primitive. One possibility is to go back to just using inexact in "library.ss" and drop $real-flonum* (keeping the "prims.ss" definition of $real->flonum). Otherwise, I don't find a better idea, but I lean toward your option 3.

[gus-massa]

I missed that there is another copy in "prims.ss". I'll take another look. In particular

(#%$real->flonum 7 7)  ; ==> error

(define (f x) (#%$real->flonum 7 x))
(f 7)  ; ==> 7.0  [I should fix this]  

[mflatt]

I considered that maybe $real->flonum could just not check it's who argument, since it's internal, and then $real->flonum in "prims.ss" can be just

(define ($real->flonum who x)  (#2%$real->flonum who x))

The worst that goes wrong with a bad who is that $oops complains. It's nice to have safe-as-possible versions of primitives, even internal ones, but the extra complication may not be worthwhile.

[gus-massa]

Sorry for the delay, I was super busy. Now I'm digging so deep that I'm worry to find the Balrog. In particular I just found this related corner case:

(define bv (make-bytevector 16))
(define (f x) (bytevector-ieee-double-native-set! bv 100 (+ x 7) bv)
(f 1) ; ==> error bytevector-ieee-double-native-set!: invalid index for 100
      ;     that is the correct error
(f 'bad) ; ==> error bytevector-ieee-double-native-set!: invalid index for 100
         ;     but it should have been
         ;     error +: 'bad is not a number

The problem is clear in the code once you notice it, so I'll fix it. But I'll need a few more days to take a deeper look.

[gus-massa]

How is it possible to make a #3% primitive that is different to the #2% version? The only method I found is to implement the #3% version in cpprims.ss and the #2% version somewhere else (like prims.ss or 5_3.ss).

But to implement the #3% version in cpprims.ss I must add float to the c-entries table [1] that is probably too much effort. An alternative is to make another primitive implemented in somewhere else (like prims.ss or 5_3.ss) that does not have a different implementation for the #2% and #3% version.

My current idea is to use the second method (that is likee the item 3 in the previos message), that is to create a new specialized primitive that is always unsafe.

[1] The table in

ChezScheme/s/cmacros.ss

Lines 3123 to 3126 in 902a100

	(declare-c-entries
	thread-context
	get-thread-context
	handle-apply-overflood

[gus-massa]

Two options with almost the same code, that just call float that was already implemented in c.

Option 1: $other-real->fixnum: It pretends that is only good for bignums and ratnums, I think it's more clrear that is a very niche function for special cases.

Option 2: $real->fixnum/slow: It's claims to be good for all reals, but slow. It's more confusing because nobody likes a slow function, but it solves some problems with the domain.

In both cases, I like that real->fixnum is included in the name, that helps a lot to grep the code to find all the variants.

[gus-massa]

[Rebase after version and submodule change. Not changes in my PR code.]

[mflatt]

The $real->fixnum/slow approach seems good to me.

I would just elaborate the !!! comment to note that applying #3%$real->flonum will check its second argument. :)

[mflatt]

Looks ready to merge after a small comment update — which I'm happy to do myself when merging, if you'd prefer.

My understanding is that there's no need for a release-note change here, because it's a repair to a new change that's already explained in the release notes. Correct me if that's not right.

[gus-massa]

Two small changes:

• Fix the !!! and use the fast path for ($real->flonum #f (something))

• Fix a helper function used in expressions that have a unique value in case it's not an error,

  (lambda (x) (when (maybe-fixnum? x) (list (exact? x)))
  ==>
  (lambda (x) (when (maybe-fixnum? x) (list (begin (exact? x) #t))))

The idea is to be more conservative to move the expression outside the tail position. I was checking unsafe or discard, but it's necessary to add more checks like arity and also because an unsafe primitive may raise an error, in particular $oops.

The function is currently used only when cptypes knows the result of the calculation, so I guess the former version can not cause any problem, I can't even write a nice problematic example. Anyway, I think it's better to fix the helper function in case I use it in other situation.

---

If you are happy with this version, feel free to squash everything and fix the comments in case it's necessary.