racket-collections icon indicating copy to clipboard operation
racket-collections copied to clipboard

unclear on reason for `gen:indexable` arity error

Open mbutterick opened this issue 7 years ago • 5 comments

This will work:

#lang racket
(require data/collection)
(struct wrapped-collection (value)
    #:methods gen:countable
     [(define (length c) 42)])

(length (wrapped-collection 'foo)) ; 42

But remove the c from the definition of length, and it won't:

#lang racket
(require data/collection)
(struct wrapped-collection (value)
    #:methods gen:countable
     [(define (length) 42)])

And the error message makes sense — length expects one argument, and we removed it:

git/racket/racket/collects/racket/private/generic.rkt:523:0: gen:countable: generic method definition
 has an incorrect arity; expected a procedure that accepts 1 argument
  length: #<procedure:length>

Now consider this example, with gen:indexable:

#lang racket
(require data/collection)
(struct wrapped-collection (value)
    #:methods gen:indexable
     [(define (ref c i) 42)])

It immediately triggers a similar arity error:

git/racket/racket/collects/racket/private/generic.rkt:523:0: gen:indexable: 
generic method definition has an incorrect arity; expected a procedure that 
accepts 1 or more arguments
  ref: #<procedure:ref>

But why? ref does in fact accept the right number of arguments.

mbutterick avatar Jun 28 '17 01:06 mbutterick

The problem seems to be in the signature of the ref generic:

  (ref indexable . _)

This notation means that the implementation of ref must have one fixed argument and one rest argument, so this amended example does work (note . i rather than i):

#lang racket
(require data/collection)
(struct wrapped-collection (value)
    #:methods gen:indexable
     [(define (ref c . i) 42)])

(ref (wrapped-collection 'foo) 'bar) ; 42

However, given that you define ref strictly as a two-ary function, this seems like an error, and that the generic definer should be:

(ref indexable idx)
(set-ref indexable idx val)

mbutterick avatar Jun 28 '17 01:06 mbutterick

Curious; it looks like those contracts are not enforced in the #:defaults block. The motivation for this in the first place was a desire to support hash-ref/dict-ref’s failure-thunk arguments, but it looks like that was a bad idea. It seems like the right thing to do here is to change the generic signature to (ref indexable index).

lexi-lambda avatar Jun 29 '17 23:06 lexi-lambda

The define-generics docs don’t mention it, but I accidentally discovered how to notate optional arguments in the generic signature:

(ref indexable idx [failure-thunk])

mbutterick avatar Jul 02 '17 18:07 mbutterick

Doing such a thing requires all implementations to support a failure thunk; my original intention was to make such support optional but allowed. Of course, this means you could not depend on an arbitrary indexable supporting a failure thunk, so you could not write indexable-generic functions that use it, but you could use the short name if you knew it was used monomorphically.

In retrospect, that’s probably not a great idea. The question just becomes whether or not all indexables should support a failure thunk or if that ought to be optional. It could theoretically be supported in a separate interface with a different name, like gen:indexable/failure and ref/failure. Do you have a strong opinion either way?

lexi-lambda avatar Jul 02 '17 18:07 lexi-lambda

my original intention was to make such support optional but allowed

I’m a generics noob. I have no opinion. Just observing that the docs are incomplete. So maybe what you originally intended is possible (by some other notational means).

mbutterick avatar Jul 03 '17 03:07 mbutterick