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

kind field for Literal #61

Closed
dead-claudia opened this issue Mar 13, 2015 · 27 comments
Closed

kind field for Literal #61

dead-claudia opened this issue Mar 13, 2015 · 27 comments

Comments

@dead-claudia
Copy link
Contributor

It is very useful for basic type checking, and IMO is a bit less hackish (and easier) than typeof node.value. Here's my idea:

interface Literal <: Node, Expression {
  type: "Literal";
  value: string | boolean | null | number | RegExp;
  // addition, the type
  kind: "string" |
    "boolean" |
    "null" |
    "undefined" |
    "number" |
    "regexp";
}
@sebmck
Copy link

sebmck commented Mar 13, 2015

I'm impartial to this since I wouldn't be able to rely on kind anyway in case the AST was mangled but I can very much see the benefits for other tooling. Also undefined shouldn't be in that list since it's an Identifier.

@dead-claudia
Copy link
Contributor Author

I filed a bug out of uncertainty (this would be a modification of the original spec).

@michaelficarra
Copy link
Member

Add "infinity" to distinguish infinity literals from other numbers? This would allow SpiderMonkey AST to be serialised to JSON. Also, agreed that "undefined" should not be there; there is no undefined literal.

@gibson042
Copy link

"infinity" seems like an abuse of this proposal. That Infinity and NaN cannot appear directly in JSON is a problem analogous to representation of ES6 regular expression literals in ES<6 runtimes.

"kind" has value on its own, but is insufficient to cover portability, which I think requires either Literal subtypes or "raw".

@michaelficarra
Copy link
Member

@gibson042 The regexp issue has already been solved by #27. NaN isn't a problem because there's no NaN literal.

@dead-claudia
Copy link
Contributor Author

I would also support a "raw" node as well.
On Mar 13, 2015 1:30 AM, "Michael Ficarra" notifications@github.com wrote:

@gibson042 https://github.com/gibson042 The regexp issue has already
been solved by #27 #27. NaN
isn't a problem because there's no NaN literal.


Reply to this email directly or view it on GitHub
#61 (comment).

@gibson042
Copy link

NaN isn't a problem because there's no NaN literal.

Is the same not true of Infinity?

@sebmck
Copy link

sebmck commented Mar 13, 2015

@gibson042

Nope, you can represent Infinity with a numeric literal. eg:

> 1e10000
Infinity

@gibson042
Copy link

Is that true in SpiderMonkey? esprima at least presents value: null in such cases, and represents both literal Infinity and NaN as Identifier.

Regardless, I maintain that kind is not the mechanism for addressing lossy representations.

@sebmck
Copy link

sebmck commented Mar 13, 2015

@gibson042 In acorn it does at least, sounds like an esprima bug.

@michaelficarra
Copy link
Member

@gibson042: No, it doesn't. You're probably observing the JSON serialisation problem we're talking about. And Infinity and NaN are identifiers. Literal infinities are just very large numbers (around 2e308 and larger) that cannot be precisely represented by an IEEE double.

If you don't think kind is a good proposal, what backward-compatible alternative do you suggest?

@gibson042
Copy link

You're probably observing the JSON serialisation problem we're talking about.

D'oh, right you are. 😳

If you don't think kind is a good proposal, what backward-compatible alternative do you suggest?

I do think kind is a good proposal, just not kind: "infinity".

@dead-claudia
Copy link
Contributor Author

I concur with kind: "infinity" being a bad idea, with backwards
compatibility being another concern.
On Mar 13, 2015 12:38 PM, "Richard Gibson" notifications@github.com wrote:

You're probably observing the JSON serialisation problem we're talking
about.

D'oh, right you are. [image: 😳]

If you don't think kind is a good proposal, what backward-compatible
alternative do you suggest?

I do think kind is a good proposal, just not kind: "infinity".


Reply to this email directly or view it on GitHub
#61 (comment).

@michaelficarra
Copy link
Member

Backward compatibility with what? Who currently implements kind?

@dead-claudia
Copy link
Contributor Author

Identifier("Infinity") versus Literal(kind: "infinity"). Tools already
depend on the former. That clarify?
On Mar 13, 2015 2:33 PM, "Michael Ficarra" notifications@github.com wrote:

Backward compatibility with what? Who currently implements kind?


Reply to this email directly or view it on GitHub
#61 (comment).

@sebmck
Copy link

sebmck commented Mar 14, 2015

@IMPinball We aren't proposing to parse the Infinity identifier as a Literal, that makes no sense. Infinite numeric literals would be given the "infinity" kind instead of "number".

@dead-claudia
Copy link
Contributor Author

Oh. Thanks for clarifying, although I don't get exactly why such a
distinction is absolutely necessary, since infinity is a number, and
checking for an infinite literal specifically isn't that hard.
On Mar 14, 2015 12:25 AM, "Sebastian McKenzie" notifications@github.com
wrote:

@IMPinball https://github.com/impinball We aren't proposing to parse
the Infinity identifier as a Literal, that makes no sense. Infinite
numeric literals would be given the "infinity" kind instead of "number".


Reply to this email directly or view it on GitHub
#61 (comment).

@sebmck
Copy link

sebmck commented Mar 14, 2015

You can't serialise an infinite numeric literal to JSON.

@RReverser
Copy link
Member

What about -Infinity, then? Do we need to add "-infinity" type for it, too?

Also, I'm not sure JSON serialization should be something that drives design of AST itself since, when it's really needed, you can implement callback for JSON.stringify that handles these edge cases.

@dead-claudia
Copy link
Contributor Author

Adds to the list of reasons why I'm not sure kind: "infinity" is really
necessary. It's easy to check for, and in reality, raw is the best way to
serialize it to JSON, anyways.
On Mar 14, 2015 12:45 AM, "Ingvar Stepanyan" notifications@github.com
wrote:

What about -Infinity, then? Do we need to add "-infinity" type for it,
too?

Also, I'm not sure JSON serialization should be something that drives
design of AST itself since, when it's really needed, you can implement
callback for JSON.stringify that handles these edge cases.


Reply to this email directly or view it on GitHub
#61 (comment).

@sebmck
Copy link

sebmck commented Mar 14, 2015

Yeah and checking the type of value is easy too.

@michaelficarra
Copy link
Member

@RReverser there are no negative number literals in JavaScript. -Infinity must be generated using unary negation.

@RReverser
Copy link
Member

@michaelficarra Depends on whether AST node was parsed or generated and inserted. In {type: "Literal", value: num} having negative num was never specified as an invalid state, and AST transformers might very well insert such nodes into the tree.

@dead-claudia
Copy link
Contributor Author

@RReverser Technically, a transformer or builder can still emit invalid ASTs. Nothing stops you from doing this, even though it's completely nonsensical.

ConditionalExpression(
  IfStatement( // invalid
    Identifier("Foo"),
    null,
    BlockStatement([])
  ),
  BlockStatement([ // invalid
    NewExpression(
      Identifier("Foo"),
      []
    )
  ]),
  Literal(0)
)
// Your invalid JS-ish result:
(if (Foo) {}) ? { new Foo(); } : 0

As for a negative numeric Literal, that would be an easy PR to make, although it would be breaking. Getting all the tools patched for that should be relatively easy, though. The Literal type does not specifiy that restriction, by the way.

@dead-claudia dead-claudia mentioned this issue Mar 14, 2015
@dead-claudia
Copy link
Contributor Author

I do feel we are digressing from the topic, though. As for the current values (without undefined, which is parsed as an Identifier), is this okay? #63

@RReverser
Copy link
Member

The Literal type does not specifiy that restriction, by the way.

That's exactly what I was pointing to by "having negative num was never specified as an invalid state".

@sebmck
Copy link

sebmck commented Mar 18, 2015

Closed via #63.

@sebmck sebmck closed this as completed Mar 18, 2015
@RReverser RReverser mentioned this issue Jun 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants