Add Enum.size #5743

bew · 2018-02-23T22:21:09Z

I just stepped on the need to know the number of enum members in an enum, and was surprised there wasn't a class method Enum.size or Enum.count, so here it is.

Sija · 2018-02-23T22:30:39Z

I'd use #size for consistent naming.

bew · 2018-02-23T22:33:15Z

I was thinking about that, and finished with: Enum is not a container, it doesn't really have a size. So count seems to be the good word for this.

makenowjust · 2018-02-24T01:42:52Z

However Enum has each method. It looks pretty like a container.

RX14 · 2018-02-24T02:44:18Z

If it's anything it's size, but can you provide a usecase?

bew · 2018-02-24T03:00:46Z

Ok, I'll change to size.

My usecase is that I have a huge enum (~400+ members) that are used as a named index in an Array (or another container), and I want to create the Array with at least the minimal size (the numbers of enum members), to make sure it won't need to realloc or anything.

So basically I want to do Array(Something).new initial_capacity: MyBigEnum.size

bew · 2018-03-10T17:25:41Z

Weird, the build failed with command bin/ci prepare_build took more than 10 minutes since last output Maybe it should be restarted?

j8r · 2018-03-20T00:14:14Z

@bew , it'll be clearer to rebase to one commit e.g Add Enum#size.

RX14 · 2018-03-20T20:00:25Z

src/enum.cr

+  # ```
+  def self.size : Int32
+    {% if @type.has_attribute?("Flags") %}
+      {{ @type.constants.size - 2 }}


Actually this should be the popcount of all the constants &ed together.

The what? what does popcount mean?

Number of non-zero bits. That's significant if there is a flag that sets multiple bits.

I'm not sure if that's the right behaviour, but generally this method is probably not that much relevant for Flag enums anyway.

For enums it should be the number of unique values the enum has (currently this is not unique), for flags it should be the number of actual flag bits covered by all the options. For example in some enums you have A = 0b01, B = 0b10 and Both = 0b11, That's 3 flag constants but only 2 flag bits, which popcount solves.

The question is, should .size return the number of constants or the number of unique values?

bew · 2018-03-28T03:04:35Z

Quoting @straight-shoota on this comment:

The question is, should .size return the number of constants or the number of unique values?

enum A
  A = 1
  B = 2
  C = 1
end

pp A::A # => A
pp A::C # => A

https://carc.in/#/r/3scm
~~It looks like duplicated values are made aliases of the original value, so the number of constant should be the same as the number of unique values! (as iirc aliases are not constants)~~

Edit: seems like I'm wrong, it's just the printing that is 'broken' as it'll print the first enum member with that value, so the number of constant is still 3 in this case, not 2.
So @straight-shoota's question still stands

bew · 2018-03-29T03:06:16Z

I don't see a valid usecase where having the number of constant is more important than the number of unique values.
So I think it should give the number of unique values.

bew · 2018-04-02T02:37:13Z

Ok I made it work for enum flags with popcount, but I didn't find a way to find the number of unique normal enum values without an "ugly" array alloc' just to count the number of unique values..

Any ideas?

jkthorne · 2018-09-05T21:03:22Z

@bew what is the status on this PR it looks like you might have some failing tests?

bew · 2018-09-07T15:29:51Z

@wontruefree I need #5903 in order to access the values of each enum fields at compile time, and get the number of unique enum values for Enum.size.

And as it currently goes (see last Ary's comment on that other issue) it's too complex and useless... 😟

If you have a good usecase for this please go ahead

bew · 2018-09-07T16:11:10Z

Actually I managed to get the values of each enum fields at compile time using a combination of @type.constants and @type.constant("Name"), shame on me I didn't really tried that earlier!!! (iirc I thought it would return the ASTNode of the constant, but it actually interprets and returns the value!)

And the problem of Field = 1 + 2 solved itself, as enum & constant top level visitor already uses the MathInterpreter I mentioned in #5903.

So there might not be any issues left, 'just waiting for the builds to finish.. 👍

Sija · 2018-09-07T16:40:29Z

spec/std/enum_spec.cr

+    end
+
+    it "gives number of unique enum values" do
+      SpecNonUniqueEnum.size.should eq 2


IMO this should count number of enum members, not values.

That's the big question of this PR I guess, but I can't find a usecase for this behavior..
Do you have one?

If the 2 behaviors are legit, maybe we can have 2 functions size & uniq_size or sth..

There's no need for uniqness if we're talking about members.

For me size should be the number of members. If you want to know the number of uniq values, select all members and call uniq.

bew · 2018-09-08T10:55:57Z

Since it seems to be the preferred behavior, I changed back Enum.size to count the number of members, should be good now

FWIW, to get uniq values, check the previous version: 9f58719

bew · 2018-12-27T17:48:42Z

One of travis build fails for memory error (the fork one), otherwise it should be good to go?
Maybe @RX14 'd want to give it another look now?

RX14 · 2018-12-27T22:17:42Z

src/enum.cr

+  # ```
+  def self.size : Int32
+    {% if @type.has_attribute?("Flags") %}
+      {{ @type }}::All.value.popcount


Do we still want to dedupe for flags, and not for non-flags enums?

It seemed to be preferable to have Enum.size return the number of members for non-flags, I guess I should change it for flags too? (the popcount can always easily be done by the user, or added later under another name)

Or even rename it to Enum.count_members to avoid any mis-interpretation?

bew · 2021-09-20T10:40:02Z

Hello, I'm cleaning up old PRs of mine on projects I don't follow anymore.

What do we do with this PR ? Close? or someone wants to take it?

straight-shoota · 2021-09-20T10:50:08Z

If you don't plan to work on this, I'd suggest to close it. It's easy to open a new PR.

Maybe we should add an issue to track this enhancement.

bew · 2021-09-20T12:32:07Z

Okay, closing it then

bew force-pushed the add-enum-count branch from 2529d17 to 99c12b1 Compare March 11, 2018 22:54

RX14 approved these changes Mar 20, 2018

View reviewed changes

RX14 requested changes Mar 20, 2018

View reviewed changes

bew force-pushed the add-enum-count branch from 2887e39 to ab3889b Compare March 28, 2018 03:05

bew changed the title ~~Add Enum.count~~ Add Enum.size Mar 29, 2018

bew mentioned this pull request Apr 2, 2018

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

Closed

bew added 2 commits September 7, 2018 17:05

Add Enum.size

a4bce3c

Give number of unique enum flags

92b15fb

bew force-pushed the add-enum-count branch from e9fb06c to 9f58719 Compare September 7, 2018 16:02

Sija reviewed Sep 7, 2018

View reviewed changes

Fix spec to assert Enum.size counts number of enum members

ac6cad0

bew force-pushed the add-enum-count branch from 9f58719 to ac6cad0 Compare September 8, 2018 10:53

Update doc

2535612

RX14 reviewed Dec 27, 2018

View reviewed changes

HertzDevil added the kind:feature label Aug 21, 2021

HertzDevil added the topic:stdlib label Aug 21, 2021

bew closed this Sep 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Enum.size #5743

Add Enum.size #5743

bew commented Feb 23, 2018

Sija commented Feb 23, 2018

bew commented Feb 23, 2018

makenowjust commented Feb 24, 2018

RX14 commented Feb 24, 2018

bew commented Feb 24, 2018

bew commented Mar 10, 2018

j8r commented Mar 20, 2018

RX14 Mar 20, 2018

bew Mar 20, 2018

straight-shoota Mar 21, 2018

RX14 Mar 21, 2018

straight-shoota Mar 21, 2018

bew commented Mar 28, 2018 •

edited

bew commented Mar 29, 2018

bew commented Apr 2, 2018 •

edited

jkthorne commented Sep 5, 2018

bew commented Sep 7, 2018

bew commented Sep 7, 2018 •

edited

Sija Sep 7, 2018

bew Sep 7, 2018

Sija Sep 7, 2018

asterite Sep 7, 2018

bew commented Sep 8, 2018 •

edited

bew commented Dec 27, 2018

RX14 Dec 27, 2018

bew Apr 4, 2019 •

edited

bew commented Sep 20, 2021

straight-shoota commented Sep 20, 2021

bew commented Sep 20, 2021

Add Enum.size #5743

Add Enum.size #5743

Conversation

bew commented Feb 23, 2018

Sija commented Feb 23, 2018

bew commented Feb 23, 2018

makenowjust commented Feb 24, 2018

RX14 commented Feb 24, 2018

bew commented Feb 24, 2018

bew commented Mar 10, 2018

j8r commented Mar 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bew commented Mar 28, 2018 • edited

bew commented Mar 29, 2018

bew commented Apr 2, 2018 • edited

jkthorne commented Sep 5, 2018

bew commented Sep 7, 2018

bew commented Sep 7, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bew commented Sep 8, 2018 • edited

bew commented Dec 27, 2018

Choose a reason for hiding this comment

bew Apr 4, 2019 • edited

Choose a reason for hiding this comment

bew commented Sep 20, 2021

straight-shoota commented Sep 20, 2021

bew commented Sep 20, 2021

bew commented Mar 28, 2018 •

edited

bew commented Apr 2, 2018 •

edited

bew commented Sep 7, 2018 •

edited

bew commented Sep 8, 2018 •

edited

bew Apr 4, 2019 •

edited