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

Reconcile differences between parsing and printing #66

Closed
1 of 3 tasks
alberdingk-thijm opened this issue May 12, 2021 · 2 comments
Closed
1 of 3 tasks

Reconcile differences between parsing and printing #66

alberdingk-thijm opened this issue May 12, 2021 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@alberdingk-thijm
Copy link
Collaborator

alberdingk-thijm commented May 12, 2021

Currently, there are a few situations in which NV expressions are parsed in one way but printed in another (see #64 for an example of this that was previously reconciled). These include:

  • operations over tuples (TGet, TSet: both not available at the user level, which is consistent with OCaml) (EDIT: set aside for now; we can reopen this issue if this becomes relevant)
  • operations over dicts (MGet: m[k] versus m at k, MSet: m[k := v] versus set(m,k,v))
  • operations over sets (union, inter and minus, which are all syntactic sugar for combine operations over dicts) (EDIT: since the AST is the same, easier to leave these as-is)

Since NV typically doesn't need to print out any NV code to the user, this discrepancy is tolerable. However, these differences do mean that there are pain points for any tools that try to use a solution from NV (handled in #64) or that want to use NV's library to generate NV code (as in nvgen). Hence, it would be worthwhile to reconcile these differences by either adding new syntax, modifying printing to match syntax more closely, or both.

In cases where syntactic sugar is used, the discrepancies may be acceptable to leave as-is, as is the case for sets versus dicts. But other cases may be useful to have handled, particularly those for dicts.

@alberdingk-thijm alberdingk-thijm added the enhancement New feature or request label May 12, 2021
@alberdingk-thijm alberdingk-thijm self-assigned this May 12, 2021
@DKLoehr
Copy link
Collaborator

DKLoehr commented May 12, 2021

IMO:

  • For dictionaries, the syntax should be m[k] and m[k := v] everywhere
  • For sets, I don't think there's an easy fix. If we really wanted to, we could rewrite the parser and AST to keep track of the different operations, but beyond that we have to keep printing them as combine operations. The resulting code should still parse back into the same AST though, so I think that's fine.
  • TGet and TSet are basically optimizations for tuple-related transformations, which is why they don't appear in the frontend syntax. I think it would be fine to add them to the parser, or to add a flag for printing out the program without them (by printing match statements instead). I'd be leery of doing that printing without a flag to disable it, though, since it's important during development that the printed version match the AST closely.

@alberdingk-thijm
Copy link
Collaborator Author

@DKLoehr That's pretty much how I felt it should be settled. I think any case where we desugar, as with sets, is fine not to fix when printing since the AST is the same. For TGet and TSet, since for now at least I'm not doing any transformations on the network before printing it out, I don't see the need to change them to print as match statements for now, although it could be nice if we could allow lets with patterns like let (u,v) = e (similar to #44 in expanding the cases where pattern matching is allowed). Either way, a flag seems sensible for that to me.

For dictionaries, I've gone ahead and written some draft code to change Printing.op_to_string to also give some information on whether the operation is prefix, infix or "circumfix" (like the brackets in the mget/mset syntax which go around their arguments: there may be a clearer neologism than circumfix), which can then be used to do printing using the consistent dictionary syntax. The only thing that looks different now is that, in situations where the operator is printed by itself (generally just in error messages like in Interp.ml or Wellformed.ml), it prints using dummy variable names as "Error in operator: a[b]" and "Error in operator: a[b := c]" instead of "Error in operator: get" and "Error in operator: set".

Additionally, I think there may be some small things I need to understand better about how the Printing module computes precedence, just to make sure I don't mix anything up, but I think that part is mostly handled.

alberdingk-thijm added a commit that referenced this issue May 13, 2021
Change printing for operations to specify how the operation is printed
in relation to its list of operands: either as a prefix, infix or
circumfix (alternating between each operand, starting with an operand).
Circumfix is only used by dict get and set for now.
It's possible some precedence ordering may have been slightly altered by
this change, but I have not had the chance yet to look through
carefully, so this is something to watch for.
Handles dict case of #66.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants