lann / wasm-wave

Web Assembly Value Encoding
Apache License 2.0
38 stars 6 forks source link

Treat `inf`, `nan`, `true`, `false` as keywords #18

Closed sunfishcode closed 10 months ago

sunfishcode commented 10 months ago

Currently there's ambiguity with inf/nan between floating-point or variant-case. We could just accept this as a quirk that we resolve using the type, however an advantage of resolving it in the grammar is that type-unaware syntax highlighters would be able to highlight inf, nan, true, and false differently from identifiers.

Consequently, I propose we interpret inf, nan, true, and false as keywords, and require a % when they're intended as identifiers.

Should we do the same for some/none/ok/err? This feels less important, because option/result despecialize to variant anyway. However, they are somewhat special, with special syntax in a few places. I'd be ok going either way on these, but have a slight preference for treating some/none/ok/err as keywords.

Thoughts?

lann commented 10 months ago

Same issue: https://github.com/lann/wave/issues/19

I'm leaning toward not treating option and result values as keywords for two reasons:

I think I'm on board with reserving at least bool and float keywords though, especially after I realized that any other option would probably orphan -inf.

sunfishcode commented 10 months ago

What's making me lean toward wanting some/none as keywords is that the situation mentioned at the bottom of the README, "a subtype-compatible change to the payload type could cause previously-encoded data to become ambiguous retroactively" would be completely avoided. Flat payloads would always work, except for the obvious option<option<T>>, and similar for result. Even result<option<T>> would work.

And, new standardized parametrized variant types seem unlikely to me. option/result clear a high bar in this context.

lann commented 10 months ago

Ah good point; that is compelling.

lann commented 10 months ago

After spending a bit of time looking at the implementation I have one concern: if these reservations apply to all labels (as currently spec'd) then they would also apply to record field names. It seems quite likely to me that someone will have an e.g. record response { ..., err: string } which would then have to be encoded as { ..., %err: "..." } which seems a bit unfortunate.

This could be avoided by only requiring (or permitting?) the % prefix on keywords where they can be ambiguous, i.e. not in record field name position. This would be a little more annoying to handle in the parser but any ambiguity would be resolved in the (forthcoming) AST and I don't think we'd lose any of the advantages here.

Thoughts?

Edit: Alternatively this could be another argument in favor of leaving option/result variant labels unreserved.

sunfishcode commented 10 months ago

My instinct here is to guess that fields named err might not be common enough to be a concern here. It will surely happen sometimes, but when it does, the tooling will have enough information to issue a clear diagnostic telling the user to add %. It could even be a fixup.

Not requiring % on record field names also sounds reasonable.

lann commented 10 months ago

20 requires %-prefixing on all keywords where used as variant or enum cases. Prefixing is not required (but is permitted) on record field and flag names.