Skip to content
This repository has been archived by the owner on Aug 14, 2019. It is now read-only.

Use spec in lib/collections expression. #1171

Closed
wants to merge 1 commit into from
Closed

Conversation

jtobin
Copy link
Contributor

@jtobin jtobin commented May 15, 2019

4b2a3d8 replaced

  =/  c=[term cord]  ((hard ,[term cord]) b)

with

  =/  c=[term cord]  ;;([term cord] b)

but without the comma, [term cord] is not parsed as a spec. As a result, collections errors on boot. This commit replaces the cell with a pair, yielding a proper spec.

4b2a3d8 replaced

  =/  c=[term cord]  ((hard ,[term cord]) b)

with

  =/  c=[term cord]  ;;([term cord] b)

but without the comma, `[term cord]` is not parsed as a spec.  As a
result, collections errors on boot.  This commit replaces the cell with
a pair, yielding a proper spec.
@jtobin jtobin requested a review from pilfer-pandex May 15, 2019 11:09
@belisarius222
Copy link

Seems like the real problem here is that ;; doesn't properly switch the parser into pattern mode when in flat form. The comma on the first sub-hoon shouldn't be necessary.

@ohAitch
Copy link
Contributor

ohAitch commented May 15, 2019 via email

@joemfb
Copy link
Member

joemfb commented May 15, 2019

As of #1164, the %mcmc parser on master does use pattern mode . @jtobin, I'm not seeing this collections failure on boot, how can I reproduce it?

@joemfb
Copy link
Member

joemfb commented May 15, 2019

I suspect you're booting into a mismatch between compiler and userspace - a compiler that doesn't have %mcmc changes and a userspace that expects them. And I further suspect that this is happening in the new cc-release pill staging scripts. The fake-ship provisioning was recently simplified to make CI work better, but it makes these mismatches more likely. Long story short, the pill update scripts need to boot ships without -A. This is my bad ....

Copy link
Contributor

@pilfer-pandex pilfer-pandex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the new ;; should parse its p (first subexpression) as a spec; if this is not happening, it should be fixed in the parser, rather than at use site like in this change, but I'm pretty sure I made that change to the parser. If joe's right, this is caused by having an old version of the compiler in a pill; if so, it's good we're discovering this now. Accordingly, I'm rejecting.

As for pair, the only difference between pair and a [] with two elems, as far as I am aware, is the addition of p and q faces. It would really be nice if we didn't have to have faces to do ordinary tuple destructuring.

@jtobin
Copy link
Contributor Author

jtobin commented May 15, 2019

Story checks out -- I observed this when using -Aon an Arvo directory that included the ;; changes, but the pill used was indeed the standard 0.7.4 one grabbed from bootstrap.urbit.org. I'll nuke this one.

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

Successfully merging this pull request may close these issues.

5 participants