cisco / ChezScheme

Chez Scheme
Apache License 2.0
6.95k stars 982 forks source link

Fix flags in signature of remove, member and assoc #686

Closed gus-massa closed 1 year ago

gus-massa commented 1 year ago

They can not be discarded in case they are comparing records with a custom equal procedure that has side effects.

burgerrg commented 1 year ago

Thanks for finding this! Do you have a short example that demonstrates the problem? If so, adding a mat for it would be helpful.

gus-massa commented 1 year ago

Something like this commit? I couldn't write a shorter version. (Note that if you test it in the current version of Chez Scheme, it fails for #3%remove, but it's ok #2%remove.)

If you like the example, I can copy it to the mats of assoc and member and squash the commits.

jltaylor-us commented 1 year ago

Seems like a reasonable test to me, and I can confirm that without the fix in this PR you get different results at optimize levels 2 and 3.

burgerrg commented 1 year ago

Thanks, the short example tests whether remove calls the equality predicate. You don't need the box:

(let ()
  (define called #f)
  (define-record-type r)
  (record-type-equal-procedure (record-type-descriptor r)
   (lambda (x y eql?) (set! called #t) #t))
  (remove (make-r) (list (make-r)))
  called)

Please use this kind of example for assoc and member. Please update the release notes by explaining that an optimization bug with these predicates was fixed when using custom record equality predicates with side effects in optimize-level 3.

gus-massa commented 1 year ago

Done.