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

Not an issue more like an optimization #147

Open
kam1986 opened this issue Apr 9, 2021 · 2 comments
Open

Not an issue more like an optimization #147

kam1986 opened this issue Apr 9, 2021 · 2 comments

Comments

@kam1986
Copy link

kam1986 commented Apr 9, 2021

In the code generated by the parser generator we see a function "tagOfToken" this can be deleted and replaced with GetHash(), which as far as I can see for all "empty" disjoint union types without attribute values just gives back the position (byte/integer) encoding just like an enum.

Havn't tested it, but I'm quit sure that a reference to some data is en general more effecient than a jump table/branching.

@teo-tsirpanis
Copy link
Contributor

Hello, GetHashCode() does not exactly do what we want. Take the following type as an example:

type MyToken =
    | STRING of string
    | INT of int
    | NULL

We want the tag of STRING "foo" to be zero, just like the tag of STRING "bar"; in other words we only care about the kind of the token, not its content. GetHashCode() does not ignore the content. Additionally, there is no guarantee that it would return zero for STRINGs, one for INTs and two for NULLs. Nor there is a guarantee that there would be no collisions. That's why we need a specialized function to get the tag of a token.

What we can do however is use reflection to more efficiently implement tagOfToken, specificaly functions of the FSharp.Reflection.FSharpValue class. If you want to try it, feel free to submit a PR or I will do it myself.

@kam1986
Copy link
Author

kam1986 commented Apr 10, 2021

I see now. did not take into account that they could carry data. forgot about %token token(s).

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

No branches or pull requests

2 participants