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

Reason V4 [Stack 3/n] [Parse Hashtags for polymorphic variants] #2614

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jordwalke
Copy link
Member

@jordwalke jordwalke commented Jul 24, 2020

Reason v4 - Parse Hashtags for polymorphic variants.


This is diff 3 in a stack of github PRs: Only examine the most recent commit in this PR to understand what this specific PR accomplishes. The whole stack is:

  1. Reason V4 [Stacked Diff 1/n Reason V4 [Stack 1/n #2605] [Allow multiple versions of Reason] #2605] [Allow multiple versions of Reason] …
  2. Reason V4 [Stacked Diff 2/n Reason V4 [Stack 2/n #2599] [String Template Literals] #2599] [String Template Literals] …
  3. YOU ARE HERE --> Reason V4 [Stacked Diff 3/n Reason V4 [Stack 3/n] [Parse Hashtags for polymorphic variants] #2614] [Parse Hashtags for polymorphic va… …
  4. Reason V4 [Stacked Diff 4/n Reason V4 [Stack 4/n #2619] [Make merlin and rtop respect latest syntax by default] #2619] [Make merlin and rtop respect late… …

Implements parsing for "hashtags" polymorphic variant constructors.
Since Reason Syntax still supports object syntax, we needed to rearrange
some syntactic real estate to make this work.

let red = #FF000;

let isRed = color => switch(color) {
  | #FF0000 => true
  | _ => false
};

let callAMethod = someObject::methodName(isRed, "testing red");

let templateLiteral = `
  String template literals are still using backticks.
  String template literals are still using backticks.
`;

Note: The top commit is the reaal change here (it's quite tiny). The bottom commits are other PRs unmerged that this is rebased on top of. For Reason Syntax v4 changes, I'll be sending out PRs, and continuously squashing each change/feature into one commit, and rebasing other PRs on top of the other PRs to form a stack. Always look at the top commit for V4 changes sent out for RFC.

Update:
I also added a cool new feature for upgrading to this diff. Even if you have not upgraded your file to the new syntax, it will detect if you use some new syntax features in the old syntax, and then use that as a hint to automatically upgrade your file (and annotate it with the new version marker)

So if you have:

let x : t(someType) = `oldPolyVariantSyntax;
let l : list<int> = [2];

Then the usage of list<int> will have refmt intelligently detect that you are trying to use a new feature, and it will automatically upgrade the entire file next time you reformat in-editor. It will print your file as:

[@reason.version 3.8];
let x : t<someType> = #oldPolyVariantSyntax;
let l : list<int> = [2];

The way this works is by having the parser be able to parse some v4 features inside of v3 (the ones that don't conflict) and detect when you use them.

So the per-file upgrade process is now:

  1. Just start using some new features like angle brackets inside a file.
  2. That's it.

@flash-gordon
Copy link

I guess polyvars for colors isn't particularly a good fit, #00FF00 won't work, will it? 😄

@@ -0,0 +1,97 @@
(**
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this comment is a duplicate of the one in the interface, I understand why you might want to have it here though, this can probably be not a doc comment

Copy link
Member Author

Choose a reason for hiding this comment

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

This diff is actually tiny, but on top of a rebase of two other PRs unmerged. The one you are discussing is actually here. Would you mind commenting on the specific PR if you have feedback?

I will likely be aggressively rebasing and force pushing to all of these PRs to keep them clean and comprehensible, where each PR's top commit is the real change for that PR with the underlying commits on the PR being other PRs.

@EduardoRFS
Copy link
Contributor

EduardoRFS commented Jul 24, 2020

@jordwalke @flash-gordon maybe we can accept poly variants starting with numbers? It's going to be really useful on a BS context.

@jordwalke
Copy link
Member Author

@flash-gordon

I guess polyvars for colors isn't particularly a good fit, #00FF00 won't work, will it? 😄

Why do you have to ruin my demo :D j/k

But yes, you are correct. I suppose there's nothing preventing us from actually forcing those identifiers to be valid in the parse tree. This diff doesn't implement that feature and I wasn't planning on implementing it but in theory it's possible. (The problem is making it interoperate with OCaml).

@jordwalke
Copy link
Member Author

jordwalke commented Jul 24, 2020

@EduardoRFS what do you mean exactly? Do you mean ##FF0000 (with two hash tags?)
You could imagine ##FF0000 would actually be parsed as something like #j82ljfxo3_3jlad8sf_ - some identifier that hashes to a runtime representation for which the first six characters are the string 'FF0000 so that at runtime, UI frameworks could just access the number directly without having to ship a huge lookup table to reverse engineer hashes to actual hex codes. The problem is - could you imagine the types of these things? I mostly meant it as a gimmicky demo, but if anyone has a solution I'm all ears.

Imagine the horrendous type:

type colors = [
|   ##000000
|   ##000001
 ....
];

I don't think OCaml would allow a type definition with that size.

A better solution may be a syntactic shortcut, that lets you implicitly pair colors together into a 3-tuple #FF#00#00 - but the allocations :/
Or something similar to the idea I had a while ago for "suffix" unit of measurements. 123Px (for pixels) which would do a type coercion to Px.t or something.

@EduardoRFS
Copy link
Contributor

@jordwalke sorry not even I know what I intended on that comment, but the point is, why not allow poly variants starting with numbers? The AST clearly supports it, and probably the compiler also supports it(I would be surprised otherwise).

It's same kinda of change as we did on let.alpha

@jordwalke
Copy link
Member Author

@EduardoRFS Yeah that's a good point. We very well could parse those identifiers. The main downside is that then we would be able to define APIs that OCaml cannot call. That is probably something to be minimized - or at the very least, there should be a large benefit to being able to define those kinds of APIs, and I think that if colors could be expressed practically, that would be one such benefit, but I cannot imagine how to make the types comprehensible. We may prefer to go another route for type safe hex colors. There may be enough syntactic real estate for postfix hashtag 00FF00# but one challenge is that on some platforms it would require a string and on others it would require an integer.

@EduardoRFS
Copy link
Contributor

Oh that's a good point, you will not be able to check it on the type system unless you make a variant with all colors, which is a terrible idea and no one should do it.

@Et7f3
Copy link
Contributor

Et7f3 commented Jul 25, 2020

In ocaml we can have 123P and it is parsed correctly but refused at type checking step so it need to be processed by a ppx. Suffix are one letter from h to z (because a-f are used for hexadecimal). We could have then 00FFFFs printed as 0x00FFFFs or ("00FFFF" [@color]) in OCaml and prefix t for tuple. Also not related but about suffix OCaml parser forbid 10pct or 10pt because suffix are 2 letters so we have to choose carefully our suffix.

@jordwalke jordwalke force-pushed the HashTags branch 2 times, most recently from b6ac0aa to 54fafb2 Compare August 6, 2020 19:50
jordwalke added a commit that referenced this pull request Aug 6, 2020
…riants]

Summary:

Implements parsing for "hashtags" polymorphic variant constructors.
Since Reason Syntax still supports object syntax, we needed to rearrange
some syntactic real estate to make this work.

```reason
let red = #FF000;

let isRed = color => switch(color) {
  | #FF0000 => true
  | _ => false
};

let callAMethod = someObject::methodName(isRed, "testing red");

let templateLiteral = `
  String template literals are still using backticks.
  String template literals are still using backticks.
`;

```

Test Plan:

Reviewers:

CC:
@jordwalke jordwalke changed the title Hash tags Reason V4 [Stack 3/n #2614] [Parse Hashtags for polymorphic variants] Aug 6, 2020
@jordwalke jordwalke changed the title Reason V4 [Stack 3/n #2614] [Parse Hashtags for polymorphic variants] Reason V4 [Stack 3/n] [Parse Hashtags for polymorphic variants] Aug 6, 2020
jordwalke added a commit that referenced this pull request Aug 10, 2020
…riants]

Summary:

Implements parsing for "hashtags" polymorphic variant constructors.
Since Reason Syntax still supports object syntax, we needed to rearrange
some syntactic real estate to make this work.

```reason
let red = #FF000;

let isRed = color => switch(color) {
  | #FF0000 => true
  | _ => false
};

let callAMethod = someObject::methodName(isRed, "testing red");

let templateLiteral = `
  String template literals are still using backticks.
  String template literals are still using backticks.
`;

```

Test Plan:

Reviewers:

CC:
jordwalke added a commit that referenced this pull request Aug 10, 2020
…riants]

Summary:

Implements parsing for "hashtags" polymorphic variant constructors.
Since Reason Syntax still supports object syntax, we needed to rearrange
some syntactic real estate to make this work.

```reason
let red = #FF000;

let isRed = color => switch(color) {
  | #FF0000 => true
  | _ => false
};

let callAMethod = someObject::methodName(isRed, "testing red");

let templateLiteral = `
  String template literals are still using backticks.
  String template literals are still using backticks.
`;

```

Test Plan:

Reviewers:

CC:
jordwalke added a commit that referenced this pull request Aug 10, 2020
…riants]

Summary:

Implements parsing for "hashtags" polymorphic variant constructors.
Since Reason Syntax still supports object syntax, we needed to rearrange
some syntactic real estate to make this work.

```reason
let red = #FF000;

let isRed = color => switch(color) {
  | #FF0000 => true
  | _ => false
};

let callAMethod = someObject::methodName(isRed, "testing red");

let templateLiteral = `
  String template literals are still using backticks.
  String template literals are still using backticks.
`;

```

Test Plan:

Reviewers:

CC:
jordwalke added a commit that referenced this pull request Aug 10, 2020
…riants]

Summary:

Implements parsing for "hashtags" polymorphic variant constructors.
Since Reason Syntax still supports object syntax, we needed to rearrange
some syntactic real estate to make this work.

```reason
let red = #FF000;

let isRed = color => switch(color) {
  | #FF0000 => true
  | _ => false
};

let callAMethod = someObject::methodName(isRed, "testing red");

let templateLiteral = `
  String template literals are still using backticks.
  String template literals are still using backticks.
`;

```

Test Plan:

Reviewers:

CC:
jordwalke added a commit that referenced this pull request Aug 10, 2020
…riants]

Summary:

Implements parsing for "hashtags" polymorphic variant constructors.
Since Reason Syntax still supports object syntax, we needed to rearrange
some syntactic real estate to make this work.

```reason
let red = #FF000;

let isRed = color => switch(color) {
  | #FF0000 => true
  | _ => false
};

let callAMethod = someObject::methodName(isRed, "testing red");

let templateLiteral = `
  String template literals are still using backticks.
  String template literals are still using backticks.
`;

```

Test Plan:

Reviewers:

CC:
jordwalke added a commit that referenced this pull request Aug 10, 2020
…riants]

Summary:

Implements parsing for "hashtags" polymorphic variant constructors.
Since Reason Syntax still supports object syntax, we needed to rearrange
some syntactic real estate to make this work.

```reason
let red = #FF000;

let isRed = color => switch(color) {
  | #FF0000 => true
  | _ => false
};

let callAMethod = someObject::methodName(isRed, "testing red");

let templateLiteral = `
  String template literals are still using backticks.
  String template literals are still using backticks.
`;

```

Test Plan:

Reviewers:

CC:
jordwalke added a commit that referenced this pull request Aug 11, 2020
…riants]

Summary:

Implements parsing for "hashtags" polymorphic variant constructors.
Since Reason Syntax still supports object syntax, we needed to rearrange
some syntactic real estate to make this work.

```reason
let red = #FF000;

let isRed = color => switch(color) {
  | #FF0000 => true
  | _ => false
};

let callAMethod = someObject::methodName(isRed, "testing red");

let templateLiteral = `
  String template literals are still using backticks.
  String template literals are still using backticks.
`;

```

Test Plan:

Reviewers:

CC:
Summary:This allows multiple versions of Reason in a single project by
inferring and recording the version of syntax used into the file in an
attribute. The attribute allows us to switch the parser and lexer on the
fly. This attribute is not the only way we can infer the version, and we
can allow project level configuration, but this is the approach that is
guaranteed to work with any build system or tooling.

Test Plan:

Reviewers:

CC:
Summary:This diff implements string template literals.

Test Plan:

Reviewers:

CC:
This reverts commit f005630051aa47c2e2a51634229e8f26d7b56ab6.
@EduardoRFS
Copy link
Contributor

ocaml/ocaml#9429 Possible problem

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

Successfully merging this pull request may close these issues.

6 participants