Add methods to get Map and EReg sizes

[ALANVF]

This pr adds 2 things:

1. size property for Map, which returns the number of entries in the map.
2. matchedNum() method for EReg, which returns the number of captured groups from the match (or throws an error if it hasn't been matched).

There is a very good chance that the eval impl won't work at first because eval's code isn't documented very well.

Before this is merged:

• https://github.com/HaxeFoundation/neko/pull/223 needs to be merged (and then ideally, release a new version of Neko).
• new version of hxcpp needs to be released (relevant pr: https://github.com/HaxeFoundation/hxcpp/pull/956 was merged).
• new version of hashlink needs to be released (relevant pr: https://github.com/HaxeFoundation/hashlink/pull/437).

[ALANVF]

Oh yeah this also implements #9303 as an added bonus

[kLabz]

Uh.. why is CI not running here? :o

Edit: ok, had to manually launch since it's your first contribution here.. weird new feature from github

[Simn]

Let's separate this into the map and EReg implementations.

Also a question: Does the Map.size implementation have linear cost on any target?

[ALANVF]

@Simn It unfortunately has a linear cost on Lua and Flash, however there's no way around it (other than to keep track of the size manually) because the languages themselves don't provide any other way to get the size (I believe it's because they both use the same hashmap implementation). I explained it in more detail here if you're curious

[Simn]

Hmm, I don't like properties that have hidden cost like that...

Also, what's with this idiotic new GitHub feature about manually running PRs, wtf...

[ALANVF]

To be fair, it's not really a hidden cost because you'd have to do it in Lua/Flash anyways (in other words, it's already a common pattern).

Re: GitHub, idek

[Simn]

To be fair, it's not really a hidden cost because you'd have to do it in Lua/Flash anyways (in other words, it's already a common pattern).

Well yes, but as a Haxe user you shouldn't have to be aware of that. I for instance clearly wasn't aware of it because I had to ask you. :D

Note that this isn't about the linear cost per-se because I understand that this cannot be avoided on these targets. My concern is that it's a property as opposed to a function because I believe that a correlation between what looks cheap and what is cheap is generally good design.

[ALANVF]

Well it is a property (or native method) on all other targets, which is why I added it as one. I suppose it can be changed to a method if needed

[back2dos]

I agree with Simn: Property access shouldn't be O(N), because that's misleading to the reader.

[jdonaldson]

I'm wary of this change since Lua is not going to have O(1) access speed like folks would expect.

I could track the size separately with another field, but this adds overhead (during updates) to a primitive instance type designed for speed.... which is also not what folks would expect.

[ALANVF]

@back2dos Noted, will change.

[ALANVF]

@jdonaldson What would you suggest then?

> Should the compiler throw an error/warning if the method is called multiple times without the map being modified? > Should defineFeature/ifFeature magic be used? > Should Lua's implementation be changed to use a custom native type? (probably a good idea in some cases, same issues as the old JS impl) > Should it be disallowed on Lua altogether?

There's no easy solution, however this should not be the sole reason why it can't be added, as all other targets support it without any issues.

[Simn]

I mean, ultimately the question becomes if we want this at all. What do people do with the size of maps? I never found this to be particularly useful.

[jdonaldson]

@jdonaldson What would you suggest then?

Should the compiler throw an error/warning if the method is called multiple times without the map being modified?

No, this would add runtime overhead... it would need to be runtime based to be 100% accurate, compiler can't tell this on its own.

Should defineFeature/ifFeature magic be used?

This is possible, the map could change behavior if it detects the method is used. However, for larger projects this behavior is a "poison pill", and could degrade performance suddenly when used... which might be surprising and difficult to track down.

Should Lua's implementation be changed to use a custom native type? (probably a good idea in some cases, same issues as the old JS impl)

This is possible, but it would then diverge from "vanilla" Lua behavior, which I would want to avoid for such a primitive type.

Should it be disallowed on Lua altogether?

That's the way I'm leaning, sadly :(

There's no easy solution, however this should not be the sole reason why it can't be added, as all other targets support it without any issues.

Lua is probably one of the trickiest targets that Haxe supports. I'm fine with making a special exception for Lua... we've done it before. However, the real question is whether this feature is useful enough on its own for other targets despite this. I'd have to defer to @simn on that one.

[ALANVF]

I personally think people would find it helpful to get the size of the map if they want to do things like:

• check if the map is empty
• verify the map has at least/no more than X number of entries
• optimize for small/large maps depending on the size

(Additionally, it seems to be used a decent amount in the compiler, so I don't see the issue here)

[Simn]

Huh, I thought we already had some sort of isEmpty on maps, but apparently we don't. In that case I'm fine with the addition. It can probably also be useful for serialization, where we have to output the numbers of elements ahead of the list.

I suppose we can address the linear Lua situation via documentation.

[Simn]

Coming back to this, I can't really remember why I said that this was good but didn't merge it. Could you update the branch?

[RblSb]

I don't think you talked about it here, maybe preparing the targets took a lot of time before haxe release (the last activity probably was in neko PR, which I fixed)

[ALANVF]

Yeah I ended up becoming really busy after setting up this pr and never got around to finishing it. At this point, I'd probably have to rebase it in order to include the new Int64Map changes, so I've been meaning to close this one and just open two separate PRs for these changes

[Simn]

Let's do the following:

1. Factor out the ES6 changes for #9303 (without the size addition) because that's a good change in its own right.
2. Factor out the size addition to all maps. This will be a little easier now with two less targets.
3. Handle the EReg change separately.

[Simn]

With #11698 merged I'll go ahead and close here because I don't expect this to be updated.