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

Restrict Enum values to numerical expression & Allow value access from macro #5903

Closed
bew opened this issue Apr 2, 2018 · 7 comments
Closed

Comments

@bew
Copy link
Contributor

bew commented Apr 2, 2018

While trying to do #5743, I noticed there's no way to get en enum member's value from macros.
I need this to be able to calculate at compile time the number of unique enum members, but if I can't access there value I can't do this.

Given the following example:

enum Foo
  One      = 1
  Two      = 1 + 1
  OneAgain = One
end

In macros, we currently have TypeNode#constants which will give the list of constant names ([One, Two, OneAgain]). I could use TypeNode#constant to access each enum member's value ASTNode, but then I have a problem with Two = 1 + 1 and OneAgain = One, I need to evaluate the value!

Discussing it with @RX14, we need something like:

  • Restrict enum values to simple numerical expressions: this should not impact anything, as enum of anything else than a number (integer?) doesn't make sense (and is already not possible iirc). I'm not sure where this restriction should happen though (after parsing? or lazily when accessing the value ?)
  • Allow to get their value from macros: so do we force-eval the value when accessing the value of an enum member from macros? For the eval part, we can probably use MathInterpreter.

What do you think?


Also, currently an enum type in macros is TypeNode, which mean we have to share TypeNode macro methods, which aren't always relevant to an Enum. I already noticed that for many other things, maybe this should be kept as a separate issue.

@jhass
Copy link
Member

jhass commented Apr 2, 2018

Why wouldn't an enum for an fixed set of strings make sense?

@bew
Copy link
Contributor Author

bew commented Apr 2, 2018

Enum is already restricted to integer type:

enum Foo : String # Error: enum base type must be an integer type
  Bar = "a"
end

puts Foo::Bar

https://carc.in/#/r/3t9l

To me it doesn't make sense because an enum's type should be a basic type, not a reference/object/etc..

@asterite may help here as you wrote the restriction?

@RX14
Copy link
Contributor

RX14 commented Apr 2, 2018

Because enums are represented as the numbers you assign as the values in the binary representation.

@asterite
Copy link
Member

This issue talks about many things. Could we have separate issues for separate topics please?

@bew
Copy link
Contributor Author

bew commented Apr 29, 2018

@asterite talks about many things: you mean in the first post? or in the comments? I don't understand what you mean exactly

@asterite
Copy link
Member

Oh, I misunderstood.

I can't see a need for Enum.size nor for this. It's too complex. Just hatdcode the number of enum members somewhere, after all it's just to save a few allocations.

@bew bew mentioned this issue Sep 7, 2018
@bew
Copy link
Contributor Author

bew commented Sep 7, 2018

Actually this issue has no raison d'etre, the feature I explain is already available in the compiler, shame on me I didn't see it...

It's a bit funny @asterite because it's you who implemented this some time ago 😃 (no offense, it's very well done, I'm learning so much from what you did on Crystal 👍 )

Const/Enum field value interpretation: 9130ec71ce

@bew bew closed this as completed Sep 7, 2018
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

4 participants