Opened 3 months ago

Last modified 3 days ago

#1581 new defect

record-instance? from (chicken memory representation) fails in modules

Reported by: kooda Owned by:
Priority: minor Milestone: 5.1
Component: core libraries Version: 5.0.0
Keywords: Cc:
Estimated difficulty: easy

Description

Since 5.0.0, we prefix record symbols with the module name, this makes the behavior of record-instance? very strange, as the same call to it can work outside a module but fail inside.

For example, this breaks the record matching of the matchable egg.

Here is a test case:

(module foo ()
(import scheme)
(cond-expand
  (chicken-4 (import chicken) (use lolevel))
  (chicken-5 (import (chicken base) (chicken memory representation))))
(define-record bar)
(assert (record-instance? (make-bar) 'bar)))

Attachments (1)

record-tag-fun.diff (6.9 KB) - added by felix 3 days ago.
Patches for coops, record-variants

Download all attachments as: .zip

Change History (4)

comment:1 Changed 3 months ago by sjamaan

It seems to me we have several ways to do this.

We can document this, so that the user will know to pass in 'foo#bar as the record name.

We could also strip the prefix when comparing, so that any bar is accepted. That means it will also accept a bar from a completely different module, which might not be desirable (but it's more consistent with the pre-5 version).

Finally, we could make it syntax somehow (or add a syntax for getting a namespaced record name) and read out ##sys#current-module.

I prefer simply documenting the current behaviour, because (chicken memory representation) is a relatively low-level module already anyway.

comment:2 Changed 3 days ago by felix

As beeing discussed on IRC there is also a problem with record-variants/coops: here structure tags are taken literally instead of the variable holding the module-decorated tag name.

Attached is a patch that modifies record-variants to use the tag variable implicitly created for a record instead of treating the record name directly as the tag (which will fail with modules). This patch also fixes some minor problems. I post this here since a few issues may be worth noting:

  • Jim should check whether my modifications of record-variants makes sense
  • define-record-variant will not work for a nonexistent record definition

The latter case appears in coops: here we use a record-variant direclty, instead of shadowing an existing record definition, this is, strictly speaking, a hack. Now, define-record-variant access the record-tag variable, which will of course no exist, so I've added a dummy record definition for coops-instance. The other coops fixes should straightforward.

Changed 3 days ago by felix

Patches for coops, record-variants

comment:3 Changed 3 days ago by felix

Some nore notes:

We should document for record-instance? that access to the tag variable is needed and suggest that it will be exported from modules.

We should check the eggs that define records for exporting the tag variables as well.

Note: See TracTickets for help on using tickets.