revert current-check-around void? contract

[stchang]

I would like to discuss reverting part of 1fbc98e

There doesn't seem to be a reason to limit the behavior, it's different from what the docs specify, and it breaks existing programs.

[samth]

cc @jackfirth

[jackfirth]

I'm open to reverting it. Could you share more information about the breakage this caused?

[stchang]

Sure. RacketScript's test suite is currently set up to have the test thunks return a value.

Long story short, since it is not checking Racket programs, it cannot operate completely using Rackunit forms, and instead it inspects and compares stdout and stderr streams returned by a test.

[jackfirth]

There's this bit in the docs:

A check checks some condition and always evaluates to (void).

I believe it was intended that checks never be allowed to return anything other than void?, so current-check-around should also never return anything other than void?. There's certainly no existing checks in rackunit that return non-void so I wouldn't be surprised if creating checks that return useful values causes parts of rackunit to break, especially if those checks return multiple values.

This was never enforced by any of the check creation forms however. We could consider changing the docs to officially allow checks to return values, but we'd need to be pretty thorough about adding tests for that.

[samth]

The 7.9 release is about to come out. I think we should definitely revert a change that breaks existing code, and do it soon.

cc @racket/release

[stchang]

Ok I see that line now.

But it still seems like void? is overly restrictive? It doesn't seem like anything would break by reverting?

Regarding multiple return values, would any/c prevent that?

[bennn]

Yep, any/c requires 1 return value. Sounds like rackunit should use this contract instead of void?

Welcome to Racket v7.8.0.5 [cs].
> (define f (contract (-> any/c) (lambda () 42) '+ '-))
> (f)
42
> (define g (contract (-> any/c) (lambda () (values 4 2)) '+ '-))
> (g)
; g: broke its own contract;
;  expected 1 value, returned 2 values
;   in: the range of
;       (-> any/c)
;   contract from: +
;   blaming: +
;    (assuming the contract is correct)
; [,bt for context]

[rfindler]

I agree that we should not let this change into the release, as it is known to be a breaking change and we don't know the limit of the damage yet

[samth]

Can we change the result contract to any, and then merge this ASAP? The release process is waiting.

[jackfirth]

Yeah, let's do that.

[stchang]

isnt that what this pr does?

[jackfirth]

Yes, I'll merge it.

(I've been a little out of it lately.)

[samth]

This PR uses any/c, which restricts the result to a single value.

[jackfirth]

Did multiple values ever work before?

[stchang]

This PR uses any/c, which restricts the result to a single value.

I though we wanted to restrict to a single value?

[samth]

Ah, ok. I'll do the release branch bit now.

[samth]

Done now.