Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add check-equal?/values and check-match/values #73

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AlexKnauth
Copy link
Member

Related to #71. I put check-equal?/values in a separate form because check-equal? is meant to be a function-like check, and adding multiple values to the existing check-equal? would require it to evaluate it's arguments differently from how a function would.

@samth
Copy link
Member

samth commented Aug 16, 2017

I don't understand why it would have to evaluate anything differently.

@AlexKnauth
Copy link
Member Author

It has to evaluate them in the context of call-with-values, so the function form can't have the same behavior.

@rfindler
Copy link
Member

What expression do you expect people to evaluate that distinguishes these two contexts? Yes, I agree there are theoretically some (things that call continuation-mark-set->context, for example), but they don't seem like something that would ever show up inside a call to check-equal?.

@jackfirth
Copy link
Collaborator

The check-eq? and check-eqv? checks need this too right? I'd find it strange if they weren't consistent with check-equal?.

What about adding a values-list syntax to define-check that let you specify that some argument positions should "capture" multiple values:

(define-check (check-equal? (values-list actual) (values-list expected))
  ... compare actual and expected, which are each lists of values ...)

;; captures the values returned by both expressions and passes them as lists
(check-equal? (values 1 2) (values 1 3))

That way, the equal checks don't need to use define-syntax and manually add check infos. And rackunit could not export the values-list syntax binding to hide this feature from clients if that was desired.

@AlexKnauth
Copy link
Member Author

But they still have to be almost "full-fledged" macros, in that they can't be function-like anymore. It won't be equivalent to say (#%app check-equal? actual-expr expected-expr) versus (check-equal? actual-expr expected-expr) if we do this.

@jackfirth
Copy link
Collaborator

jackfirth commented Aug 29, 2017

I think it's a reasonable tradeoff. It's purely additive; no existing uses of check-equal? would break as a result of adding this feature. Even existing higher order uses will continue to work as they always have.

@AlexKnauth
Copy link
Member Author

AlexKnauth commented Aug 29, 2017

And what if someone is testing functions that look at continuation-mark stuff?

(Edit: I can't find a way to detect it other than looking at the error message, so far.)

@jackfirth
Copy link
Collaborator

Would that work now?

@rfindler
Copy link
Member

I don't think that supporting people writing (#%app check-equal? ...) is important, compared to being able to support people writing (check-equal? (f 1) (values 1 2)) for functions that return multiple values. Not even close!

And the earlier distinguishing context I gave was meant to reenforce how there aren't any interesting contexts that can distinguish them.

@AlexKnauth
Copy link
Member Author

AlexKnauth commented Aug 29, 2017

Here's something that distinguishes them:

#lang racket/base

(require rackunit)

(define fn-context (#%plain-app make-continuation-prompt-tag 'fn-context))

(define-syntax-rule (#%app f a ...)
  (#%plain-app (with-continuation-mark fn-context 'function-position f)
               (with-continuation-mark fn-context 'argument-position a)
               ...))

(define (stuff)
  (let/cc k
    (continuation-mark-set->list (continuation-marks k) fn-context)))

;; this passes:
(#%app check-equal? (stuff) '(argument-position))
;; this fails:
(check-equal?/values (stuff) '(argument-position))

(Edited to leave out unnecessary call-with-values)

@rfindler
Copy link
Member

Well, of course that does. If you write (m check-equal? a b) you can make arbitrary changes to things. Here's another example that is just the same as yours and not something that I think is very relevant to this discussion.


#lang racket
(require rackunit)
(define-syntax-rule
  (m x ...)
  (printf "I am different because a macro rewrote me!\n"))

(m check-equal? 1 2)
(check-equal? 1 2)

@rfindler
Copy link
Member

Put another way, in case what I wrote was not clear: your example is not really about continuation marks. Here's another example to illustrate this point.

#lang racket
(require rackunit)

(define global 1234)

(define-syntax-rule
  (m f x ...)
  (f (let ([saved-value global])
       (set! global 15)
       (begin0 x
               (set! global saved-value)))
     ...))

(m check-equal? global 1234)

(check-equal? global 1234)

I don't think this is relevant to check-equal? at all.

@jackfirth
Copy link
Collaborator

jackfirth commented Aug 29, 2017

Do (#%app check-equal? (stuff) '(argument-position)) and (#%app check-equal?/values (stuff) '(argument-position))) have different results?

@AlexKnauth
Copy link
Member Author

Okay. I just realized check-equal? itself doesn't use the #%app from its use site but the app from its definition site.

@AlexKnauth
Copy link
Member Author

(Re: @jackfirth's comment)
The check-equal?/values form wouldn't make sense as a function expression because functions can't accept multiple-valued arguments. I didn't want to pretend that check-equal?/values could be used as a function so I made it a pure macro.

@jackfirth
Copy link
Collaborator

jackfirth commented Aug 29, 2017

I don't see why it can't act as a function when used higher order, it would just lose its magical value-capturing abilities (and any "value-list" argument would be a list of one value)

@rfindler
Copy link
Member

@AlexKnauth I see what you mean now: if check-equal? picked up the #%app from the use site, then your examples and line of inquiry make sense to me.

But even if it did, I'd say that changing it is still a good idea. I don't think that examples like you posted are ones that we need to worry about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants