Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor conventional capdata body literals #2247

Closed
FUDCo opened this issue Jan 23, 2021 · 9 comments
Closed

Refactor conventional capdata body literals #2247

FUDCo opened this issue Jan 23, 2021 · 9 comments
Assignees
Labels
hygiene Tidying up around the house SwingSet package: SwingSet

Comments

@FUDCo
Copy link
Contributor

FUDCo commented Jan 23, 2021

At various points in the SwingSet code there are strings such as '{"@qclass":"slot","index":0}', representing the body portion of a capdata object that is being synthesized for use in some conventional role. We should factor out this string, or the pre-JSON-serialized form, into a standard constant somewhere. Or maybe into a function that takes an object reference string and returns a single-object capdata for it.

@FUDCo FUDCo added SwingSet package: SwingSet hygiene Tidying up around the house labels Jan 23, 2021
@FUDCo FUDCo self-assigned this Jan 23, 2021
@erights
Copy link
Member

erights commented Jan 23, 2021

I do not understand what this is saying. I'm not objecting. I just don't understand.

@FUDCo
Copy link
Contributor Author

FUDCo commented Jan 23, 2021

It's a magic string constant that incorporates particular details of our serialization format. It would be cleaner if it were either synthesized from actual serialization or at least named on one place so that it could be more easily updated if the serialization format changes.

@erights
Copy link
Member

erights commented Jan 23, 2021

Ok sure. But with one exception, I see {"@qclass":"slot","index": appear literally only in tests.

@FUDCo
Copy link
Contributor Author

FUDCo commented Jan 23, 2021

This is not a high priority cleanup, which is why it's its own ticket.

@erights
Copy link
Member

erights commented Jan 23, 2021

;)

@FUDCo
Copy link
Contributor Author

FUDCo commented Jan 23, 2021

To quote Brian Terlson, "we all have bugs we're never going to fix, that's why we file 'em".

@warner
Copy link
Member

warner commented Jan 23, 2021

@erights We're transitioning away from three separate resolution API calls (fulfillToPresence, fulfillToData, reject(to data), where the first takes a single bare slot identifier (the source of your lost-interface-name problem) and the other two taking capdata. We're transitioning to a single API call resolve that takes a (list of) two-tuple of a reject boolean (false for fulfill, true for resolve and eventually forward), and capdata.

The transition is not yet complete, so there are places where we have to map from one API to the other. When the mapping receives a fulfillToPresence(slotid), it must emit a resolve(false, capdataForOnePresence(slotid)). Because this happens in the kernel or in the comms vat, where marshal is not available, and because it's so stereotypical, we're building it by hand.

It's quite possible that once we finish the transition, we won't have any occurrences of this pattern left and we won't need the refactoring.

There are two cases I can think of where the kernel needs to synthesize the result of a failed message send (messages delivered to terminated vats, and messages queued to a promise which is then resolved to a non-presence). Those might want a similar kind of refactoring, but the function would create the serialized representation of an Error object instead of a presence.

Ideally, the kernel would be completely unaware of how vats do their serialization. To accomplish this, perhaps the orphaned exports of terminated vats could be adopted by a special "error vat" which reacts to all messages by throwing an error. I'm not sure that would help with the queued-message-resolves-to-data case, though, and I'm not sure the overhead would be worth the nominal tidyness.

@erights
Copy link
Member

erights commented Jan 23, 2021

two-tuple of a reject boolean (false for fulfill, true for resolve and eventually forward), and capdata.

Is this what you meant to say? In the semantics of promises, the taxonomy is

  • unresolved
  • resolved
    • forwarded
    • settled
      • rejected
      • fulfilled

so the leaf cases of resolved are forwarded, rejected, or fulfilled. I'll wait for clarification on this before responding to the rest.

@FUDCo
Copy link
Contributor Author

FUDCo commented Jan 25, 2023

Smallcaps renders this irrelevant. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hygiene Tidying up around the house SwingSet package: SwingSet
Projects
None yet
Development

No branches or pull requests

4 participants