[and/c | or/c]: ignore `any/c` | `none/c`. #4437

NoahStoryM · 2022-09-21T22:40:36Z

Before:

Welcome to Racket v8.6 [cs].
> (or/c #t #f (or/c))
(or/c #t #f (or/c))
> (or/c #t #f (or/c) 'x)
(or/c #t #f (or/c) 'x)
> (first-or/c #t #f (first-or/c) 'x)
(first-or/c #t #f (first-or/c) 'x)
> (and/c any/c)
(and/c any/c)
> (and/c any/c any/c)
(and/c any/c any/c)
> (and/c number? (and/c) integer?)
(and/c number? any/c integer?)

After:

Welcome to Racket v8.6 [cs].
> (or/c #t #f (or/c))
boolean?
> (or/c #t #f (or/c) 'x)
(or/c #t #f 'x)
> (first-or/c #t #f (first-or/c) 'x)
(first-or/c #t #f 'x)
> (and/c any/c)
any/c
> (and/c any/c any/c)
any/c
> (and/c number? (and/c) integer?)
(and/c number? integer?)

And the path error in contract/private/types.rkt seems to have no effect on TR, does the current version of TR still need this file to set up information?

By the way, I didn't find define/subexpression-pos-prop/name in the document.

rfindler · 2022-09-21T22:51:04Z

Should (or/c) come out as none/c? (Then maybe some of these changes will amount to just looking for none/c?)

NoahStoryM · 2022-09-21T23:14:19Z

Should (or/c) come out as none/c? (Then maybe some of these changes will amount to just looking for none/c?)

Treating (or/c) and (first-or/c) as none/c has no effect since nones (none/c, (or/c), (first-or/c)) are ignored by remq*. And I'm not sure if this change will work, maybe (or/c) and (first-or/c) are used somewhere.

rfindler · 2022-09-22T01:18:52Z

I'm not following the comment about remq. And if (or/c) is used somewhere, then that seems fine; it accepts the same values as none/c, right?

NoahStoryM · 2022-09-22T04:13:35Z

I mean maybe some places already use '(or/c) as contract-name. And on the other hand racket provides make-none/c to make new none, so I guess making (or/c) evaluate to none/c may affect some code.

rfindler · 2022-09-23T00:51:45Z

That's definitely true. We could do something similar to the way that any/c works (c.f. named-any/c), where there are many "names" for none/c but all of answer true to none/c? (or some other predicate) and then use that predicate when removing them from arguments?

NoahStoryM · 2022-09-23T02:50:08Z

For first-or/c this change I think is feasible.

But if or/c does the same thing, I don't know how to make (opt/c (or/c ...)) return the same name as (or/c ...):

Welcome to Racket v8.6 [cs].
> (or/c 1 2 (make-none/c 'any/c))
(or/c 1 2)
> (opt/c (or/c 1 2 (make-none/c 'any/c)))
#<opt-chaperone-contract: (or/c 1 2 any/c)>

NoahStoryM · 2022-09-23T04:38:08Z

racket/collects/racket/contract/private/misc.rkt

@@ -666,34 +665,21 @@
  (define env (contract-random-generate-get-current-environment))
  (λ () (random-any/c env fuel)))

-(define-struct any/c ()
+(define-struct any/c (name)


Before:

Welcome to Racket v8.6 [cs]. > (object-name (or/c any/c any/c any/c)) 'named-any/c

After:

Welcome to Racket v8.6 [cs]. > (object-name (or/c any/c any/c any/c)) 'any/c

I'm not really sure what's going on here (I can't tell from what I'm reading on github), but I see a diff where a name field was added to any/c and I don't think we want that. We should stick with using named-any/c if we want a name.

NoahStoryM · 2022-09-23T04:51:39Z

I wonder if racket/contract is willing to provide make-any/c (as the dual of make-none/c) ?
Thus premissive/c can be described as:

If (permissive-xexprs) is #t, then equivalent to (make-any/c 'permissive) ~~any/c~~, otherwise equivalent to (make-none/c 'permissive).

And also any/c? and none/c? ?

rfindler · 2022-09-23T14:55:04Z

I wonder if racket/contract is willing to provide make-any/c (as the dual of make-none/c) ?

I don't think that's necessary.

My thinking is that (or/c) would actually return none/c but the name of that none/c would be still be (or/c). So we'd have (contract-name (or/c)) returning '(or/c) even though, internally, it is a none/c.

This is the setup for the way any/c works for exactly the reasons that you're talking about above.

NoahStoryM · 2022-09-23T15:06:33Z

It makes sense to me. And what about none/c?? Should we provide it?

rfindler · 2022-09-23T18:02:56Z

I think none/c? is an internal detail for now. But if there is some contract outside the contract library can benefit from it the way or/c is, then we could provide it. There is always a worry about adding extra names to widely-used libraries, tho.

rfindler · 2022-09-24T23:31:12Z

racket/collects/racket/contract/private/prop.rkt

@@ -620,7 +620,7 @@
  (make-struct-type-property 'prop:any/c))

 ;; this property's value isn't looked at; it is just a signal
-;; that the contract accepts any value
+;; that the contract accepts none value


Maybe "no" instead of "none"?

racket/collects/racket/contract/private/and.rkt

rfindler · 2022-09-24T23:35:44Z

racket/collects/racket/contract/private/and.rkt

+    [() any/c]
+    [raw-arg*
+     (define raw-args (remove-duplicates (filter-not prop:any/c? raw-arg*) eq?))
+     (case (length raw-args)


Let's not do this. Instead do (null? (cdr contracts)) as a new second case.

rfindler · 2022-09-24T23:37:01Z

racket/collects/racket/contract/private/misc.rkt

@@ -666,34 +672,18 @@
  (define env (contract-random-generate-get-current-environment))
  (λ () (random-any/c env fuel)))

-(define-struct any/c ()
+(struct any/c ()


Is removing define- here necessary for the rest of the things that are happening in this PR? If not, let's leave it as is. (I would like to keep the history very clean here as I have used it in the past and it's been helpful.)

rfindler · 2022-09-24T23:37:28Z

racket/collects/racket/contract/private/misc.rkt

@@ -706,19 +696,20 @@
   (none/c-name ctc)
   val))

-(define-struct none/c (name)
+(struct none/c (name)


Ditto for this change.

rfindler · 2022-09-24T23:38:05Z

racket/collects/racket/contract/private/misc.rkt

  #:property prop:flat-contract
  (build-flat-contract-property
   #:trusted trust-me
   #:late-neg-projection none-curried-late-neg-proj
-   #:stronger (λ (this that) #t)
-   #:equivalent (λ (this that) (none/c? that))
+   #:stronger (λ (this that) (prop:none/c? that))


I think this change is incorrect. If there aren't test cases that this change breaks, let's add some.

rfindler · 2022-09-24T23:40:09Z

racket/collects/racket/contract/private/orc.rkt

-     (cond
-       [(ormap prop:any/c? args) (named-any/c (contract-name the-or/c))]
-       [else the-or/c])]))
+  (let ([none? (make-none/c '(or/c))])


I don't think adding this let is a good idea. We are already using none? as somethign elsewhere here and it is a different thing than this. Also, the let change the indentation, making the diff more painful to work through; in this case we can do this without an indentation change by using define.

rfindler · 2022-09-24T23:40:35Z

racket/collects/racket/contract/private/orc.rkt

-       [(andmap chaperone-contract? args)
-        (make-chaperone-first-or/c args)]
-       [else (make-impersonator-first-or/c args)])]))
+  (let ([none? (make-none/c '(first-or/c))])


Ditto here with the let.

rfindler · 2022-09-25T12:34:33Z

racket/collects/racket/contract/private/orc.rkt

+  (case-lambda
+    [() or/c:none/c]
+    [raw-arg*
+     (define raw-args (remq* (list none/c or/c:none/c (first-or/c)) raw-arg*))


Why not use a none/c? predicate here?

Because (or/c ...) and (opt/c (or/c ...)) should have the same name.
#4437 (comment)

I'm not following the answer. Definitely it should be the case that opt/c is less important than the regular use, so we should improve opt/c. Can you say more about what's going on here?

The contract-name of (opt/c (or/c ...)) is checked in the test file name.rkt.

So if we use none/c?, this test will fail:

> (contract-name (or/c (make-none/c 'none) #t #f)) 'boolean? > (contract-name (opt/c (or/c (make-none/c 'none) #t #f))) '(or/c none #t #f) > (test-name 'boolean? (or/c (make-none/c 'none) #t #f)) FAILED (#<procedure:contract-name> #<opt-chaperone-contract: (or/c none #t #f)>) got (or/c none #t #f) expected boolean?

Here is the definition of the name of (opt/c (or/c ...)) .

The two options are to adjust the test case to avoid checking opt/c or to improve opt/c to recognize this case and drop the call to make-none/c. I'm happy either way. I think the code that was there, dynamically doing the removal, isn't the best choice, so probably better not to make that more complicated. (If it has to get more complicated, it should be moved into a helper function and a call to the helper function should be generated.)

I haven't figured out how to improve opt/c, so I just avoid checking it.

samth · 2024-01-08T18:24:49Z

What's the current status of this PR?

[and/c | or/c]: ignore any/c | none/c.

e89d6c6

make and/c (first-or/c) ignore all kinds of any/c (none/c).

a8e20bb

NoahStoryM commented Sep 23, 2022

View reviewed changes

NoahStoryM added 2 commits September 23, 2022 13:37

Update.

044c1a4

[and/c | first-or/c] : remove duplicate predicates.

97b544d

NoahStoryM and others added 2 commits September 24, 2022 07:14

revert any/c structure.

02dc62b

Fix typo.

6360d46

rfindler reviewed Sep 24, 2022

View reviewed changes

racket/collects/racket/contract/private/and.rkt Show resolved Hide resolved

rfindler reviewed Sep 24, 2022

View reviewed changes

NoahStoryM added 5 commits September 25, 2022 19:42

typo.

1a21b6d

Use (cond [(null? ...)]) instead of (case (length ...)).

611018e

Use define-struct instead of struct.

073157e

Fix none/c-flat-contract-property.

478153d

Use define new none/c via define instead of let.

3a35a70

NoahStoryM added 2 commits September 25, 2022 20:17

Fix conflict constructor names.

b58a2f2

stronger? test.

7788fb6

rfindler reviewed Sep 25, 2022

View reviewed changes

NoahStoryM added 4 commits September 25, 2022 20:43

Update.

ca01f13

Update.

ac18e48

More tests.

2c28c6f

add a test.

0be5a4c

NoahStoryM force-pushed the andor branch from b4813ea to 0be5a4c Compare September 25, 2022 21:24

Avoid checking opt/c.

ac6e320

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[and/c | or/c]: ignore `any/c` | `none/c`. #4437

[and/c | or/c]: ignore `any/c` | `none/c`. #4437

NoahStoryM commented Sep 21, 2022

rfindler commented Sep 21, 2022

NoahStoryM commented Sep 21, 2022

rfindler commented Sep 22, 2022

NoahStoryM commented Sep 22, 2022

rfindler commented Sep 23, 2022

NoahStoryM commented Sep 23, 2022

NoahStoryM Sep 23, 2022

rfindler Sep 23, 2022

NoahStoryM commented Sep 23, 2022 •

edited

rfindler commented Sep 23, 2022

NoahStoryM commented Sep 23, 2022

rfindler commented Sep 23, 2022

rfindler Sep 24, 2022

rfindler Sep 24, 2022

rfindler Sep 24, 2022

rfindler Sep 24, 2022

rfindler Sep 24, 2022

rfindler Sep 24, 2022

rfindler Sep 24, 2022

rfindler Sep 25, 2022

NoahStoryM Sep 25, 2022

rfindler Sep 25, 2022

NoahStoryM Sep 25, 2022 •

edited

NoahStoryM Sep 25, 2022

rfindler Sep 26, 2022

NoahStoryM Nov 3, 2022

samth commented Jan 8, 2024

[and/c | or/c]: ignore any/c | none/c. #4437

Are you sure you want to change the base?

[and/c | or/c]: ignore any/c | none/c. #4437

Conversation

NoahStoryM commented Sep 21, 2022

rfindler commented Sep 21, 2022

NoahStoryM commented Sep 21, 2022

rfindler commented Sep 22, 2022

NoahStoryM commented Sep 22, 2022

rfindler commented Sep 23, 2022

NoahStoryM commented Sep 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NoahStoryM commented Sep 23, 2022 • edited

rfindler commented Sep 23, 2022

NoahStoryM commented Sep 23, 2022

rfindler commented Sep 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NoahStoryM Sep 25, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samth commented Jan 8, 2024

[and/c | or/c]: ignore `any/c` | `none/c`. #4437

[and/c | or/c]: ignore `any/c` | `none/c`. #4437

NoahStoryM commented Sep 23, 2022 •

edited

NoahStoryM Sep 25, 2022 •

edited