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

Spec draft tokens #1

Open
wants to merge 1 commit into
base: spec-draft
Choose a base branch
from
Open

Spec draft tokens #1

wants to merge 1 commit into from

Conversation

Yoric
Copy link
Owner

@Yoric Yoric commented Oct 2, 2018

Hey, @syg, could you take a look at this?

By definition, `typedef` is strictly an alias for a type. Therefore,
the following sequence mean that `IdentifierName` and `Identifier`
are interchangeable:
```
typedef string IdentifierName;
typedef string Identifier;
```

As this is not true, the current patch is a first attempt to remove this aliasing.

We solve this by introducing an annotation `[Token(...)]`, which lets us specify
the token-space in which an alias of string belongs.

So,

```
typedef [Token(IdentifierName)] string IdentifierName;
typedef [Token(Identifier)] string Identifier;
```

specifies that an `IdentifierName` is a string from the set of tokens `IdentifierName`.

This is an early draft for feedback, we haven't yet actually defined the set of tokens.
@syg
Copy link

syg commented Oct 2, 2018

Sorry I seem so obstinate about this: I still don't understand the motivation very well. Why is it undesirable to have the implementation understand the, e.g. the moduleSpecifier inside an Import node is to be treated differently than a plain identifier use? The immediate surrounding node type should be readily available information, right? Building this information into the node type still seems redundant and is more about encoding semantics than syntax.

@Yoric
Copy link
Owner Author

Yoric commented Oct 3, 2018

For one thing, we have the problem that typedef string Foo; typedef string Bar; actually means that Foo and Bar are exactly the same thing. So, we cannot decide that Foo has some restrictions but Bar doesn't. So, I tend to believe that before being useful, this fixes an actual spec error.

Now, I find your example undesirable because it forces us to introduce hardcoded logics specifically to work around the fact that we use the same type for unrelated pieces of data. In any programming language, if I have different types of data, I'm going to newtype them, I just want to do the same here :)

@syg
Copy link

syg commented Oct 3, 2018

Now, I find your example undesirable because it forces us to introduce hardcoded logics specifically to work around the fact that we use the same type for unrelated pieces of data.

I agree that for string literals with different lexical grammars, this type differentiation is desirable. On the other hand I think differences beyond grammar differences don't need to be encoded as different types in the AST tree grammar. Do you agree with that?

For example, one can argue that an identifier use of a local binding is different than an identifier use of an outer binding, or that a function statement that contains a recursive use is different than a function statement that is non-recursive. I am explicitly not interested in encoding semantic differences of that kind.

@Yoric
Copy link
Owner Author

Yoric commented Oct 4, 2018

I can go with that.

@Yoric
Copy link
Owner Author

Yoric commented Oct 9, 2018

@syg So, what's your opinion on this specific patch?

@Yoric
Copy link
Owner Author

Yoric commented Oct 10, 2018

An alternative would be to wrap them in interfaces instead

interface Identifier {
  value: string;
}
interface IdentifierName {
  value: string;
}
interface Label {
  value: string;
}
interface IdentifierLikePropertyNameLiteral {
  value: string;
}
interface PropertynameLiteral {
  value: string;
}
interface ModuleSpecifierLiteral {
  value: string;
}
interface RegExpFlags {
  value: string;
}
interface StringLiteral {
  value: string;
}

with specifications for the correct values for each of these value fields.

Either choice will require some work on the implementation side.

@syg
Copy link

syg commented Oct 10, 2018

I prefer the interface wrapping for now.

@Yoric
Copy link
Owner Author

Yoric commented Oct 10, 2018

Ok, that works. I'll reformulate this as an interface.

Then I'll spend a few days trying to get the code to work with that :)

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 this pull request may close these issues.

2 participants