Urbit: +slab is broken

Created on 6 May 2020  路  9Comments  路  Source: urbit/urbit

+slab, the standard library arm that purports to produce a flag indicating whether a limb exists in a type, is broken in two ways. First, it fails to honor core variance, yielding false positives. Second, in any case where it should produce %.n, it crashes instead.

Consider a lead core with one arm, +foo. Trying to access anything other than 'foo' from this core should fail, and it does:

> =<  zing  ^?  |%  ++  foo  0  --
-find.zing

However, +slab, which purports to produce a flag indicating whether a limb exists in a type, cheerfully reports that zing can be called, since it would be accessible if the core variance rules were ignored (+zing is in the standard library, which became this core's context due to the |%):

> %+  slab  'zing'  =<  p  !>  ^?  |%  ++  foo  0  --
%.y

If you call +slab with anything that isn't in the type, it just crashes, instead of producing %.n. Consider a core with the same battery, but without the standard library:

> %+  slab  'zing'  =<  p  !>  ^?  =>  ~  |%  ++  foo  0  --
-find.zing
bug hoon

Most helpful comment

What does slob do, exactly? I got as far as q.q.n.q.r.q.typ and then ragequit.

All 9 comments

Considering this and the error I found recently in +sloe, I think we might want to add some unit tests on these vase and type manipulation functions.

I should note that +slob does work, so it's possible I'm misunderstanding how +slab should be used. It still doesn't seem right, though.

What do you think are the correct semantics for handling variance in slab? You say that slab should say whether a limb "exists" in a type. (The doc comment on slab, such as it is, instead uses the equally unhelpful word "contains.") Obviously, the limbs in the context of a lead, zinc, or iron core should not exist. But what about the sample of a zinc or iron core? Well, they "exist" for reads but not writes (or vice versa). So they kind of half exist. I can see three options here:

  1. It exists if you can read OR write it.
  2. It exists if you can read it.
  3. It exists if you can write it.
  4. It exists if you can read AND write it.

I think the most likely correct answer is 2, because I'd expect this operation to be used in a context where you have a vase, and you want to read a limb if it's there and otherwise do something else. But note that this is not really a satisfying answer.

The variance semantics are implemented in +peel, which takes a ?(%read %rite %both %free) and a metal and returns a flag saying whether you can perform the operation in question on the sample and a flag likewise on the context. The current implementation of slab calls find with %free, which is the "totally ignore variance" option (it returns [& &]), giving the incorrect behavior you complain about. Of the other choices, %read gives us 2, %rite gives us 3, %both gives us 4, and there is no way of getting 1 without calling find twice.

Another option would be to make the caller pass in a ?(%read %rite %both)

What does slob do, exactly? I got as far as q.q.n.q.r.q.typ and then ragequit.

"Has in the core itself", looks like? $core has a ++coil which has a map of chapters by name(oh hey chapters have names now!); looks like q.r.q.typ is that map, so presumably the q of the q.n of the q.r.q.typ being iterated over is the map of arms in that chapter.

Yes, +slob answers the question of whether a core type has an arm of the specified name. It's not a general environment lookup like +slab.

As for how +slab should work, I think that depends on the use case. Once Ford Fusion lands, there will be no calls to this function at all, so maybe we should just delete it. It does seem like a basic utility, though, so if we keep it around I think it should take in the variance as an argument and pass that through into the +peel call. That way, the caller would have to know which variance it wants, which I think is appropriate.

Great, I think this is the best option too. I'll make a pr to that effect on top of ford-fusion.

Closing, as this #2923 released with ford fusion (merged in 3daecd8c1).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

hrharder picture hrharder  路  10Comments

vvisigoth picture vvisigoth  路  6Comments

kirkins picture kirkins  路  6Comments

custom-jonathan picture custom-jonathan  路  10Comments

timlucmiptev picture timlucmiptev  路  4Comments