Discussion: Incorporating Erlang/OTP 21 map guards in Elixir

[AnilRedshift]

With respect to Discussion #3 (map.field in guards)

I would love for this to not be a guard, but rather syntactic sugar.

e.g.

def serve_drinks(user) when user.age >= 21 do
  ...
end

would generate this under the covers:

def serve_drinks(%{age: __age__} = user) when __age__ >= 21 do
  ...
end

That would allow you to get perf benefits of pattern matching without having to write the destructuring syntax. Obviously that has issues for injecting a variable to the underlying code. However

1. There’s no back-compat issues because the original code doesn’t compile today
2. reserving __X__ variables for elixir doesn’t seem like the end of the world. (and you can namespace however you want)

[michalmuskala]

This is not possible to do. Consider:

def foo(user) when is_binary(user) or user.age >= 21

On the other hand, “reserving variables” wouldn’t be a problem - we have hygienic macros that already solve that issue.

[LostKobrakai]

On #1:
Yes, because of …

• Completeness and Discoverability
  People’s first place to look for guards are the Kernel docs. @michalmuskala told me there are some exceptions to that already, but they all seem to have their specific reasoning for not being there.
• Naming/Argument-Order
  What José mentioned here: Discussion: Incorporating Erlang/OTP 21 map guards in Elixir

On #2:
Yes, but as @axelson mentioned: It should support at least the majority if not all of normally possible patterns.

On #3:
No. There are already places where such calls are not allowed like the following, so this should be consistently disallowed in guards as well.

entity = %{id: 1}
%{id: ^entity.id} = entity

[AnilRedshift]

def foo(user) when is_binary(user) or user.age >= 21

could be sugar’d as such:

def foo(user) when is_binary(user) do
  ...
end
def foo(%{age: __age__}) when __age__ >= 21 do
  ...
end

With a limited number of guard clauses, it wouldn’t be too hard to special case out the type-check guard clauses.

[christhekeele]

My 2 cents:

---

Discussion #1: Should we add map_get/2 and is_map_key/2 to Kernel ?

Yes! I think we should, I would find it surprising if some were missing. Right now browsing the Kernel guards list is how newcomers learn what is possible in guards. Plus if we keep the same names it will reduce cognitive dissonance when composing matchspecs, where map_get/2 will really shine.

@dimitarvp

No, it breaks the “only one way to do thing X” rule that won so much fans of Elixir and, partially, Go.

This was never a huge consideration for me IMO.

---

Discussion #2: Should we allow patterns in defguard/1 ?

No. I think that would be great, but @axelson make a very compelling point:

@axelson

If we can’t support all of pattern matching (with the existing semantics) in defguard/1 then I think we shouldn’t allow it at all.

Just like allowing Map.fetch in guards would be an inconsistent exception to the rules, I think all-or-nothing is important here. Guards are already semantically special cases and we don’t want to add to that.

---

Discussion #3: Should we allow map.field in guards?

I vote no, for the misleading mod.fun remote call confusion you describe.

---

All in all my opinions are motivated by consistency and minimal surprise to newcomers:

1. Yes
2. No
3. No

[Eiji]

@josevalim

In my opinion, the real advantage of map_get/2 and is_map_key/2 is that we can use them to build composable guards. For example, now we can finally implement guards such as is_struct/1 :

I agree with that point.

@josevalim

Discussion #1: Should we add map_get/2 and is_map_key/2 to Kernel?

Normally I would say yes, but keeping previous thing in mind I think that there is no need.

I would say yes if there will be use case for them in real function, so for example I can easily pipe such functions, but in this case I do not see a real need to do that.

@josevalim

Discussion #2: Should we allow patterns in defguard/1 ?

This is awesome idea! It would simplify lots of parts of my code! However I think that it should be separate function like gen_guard (generate guard), because I believe that lots of newbies are confused enough with “normal” guard declarations. Also when adding guards support I (as a definitely non-expert) found few bugs for just only this one feature, so I’m a bit worried whenever guard handling code would be too complicated.

@josevalim

Discussion #3: Should we allow map.field in guards?

Hmm, as you said I think that well used would be awesome feature. I would say that if you believe that your FunctionClauseError messages are well written for all of that weird edge-cases then go for it! Anyway we have forum and community to improve messages if we find that they are confusing.

[radarjones]

#1 Yes
#2 Yes
#3 No

[blatyo]

1. Yes (elixir convention names)
2. Not sure
3. No

[aethereus]

The map_get/2 and is_map_key/2 guards

I can’t say no, but I think we can postpone it until someone finds a real use case that these features are a must.

Expand defguard

A big YES.

Support map.field in guards

No. The map.field may raise errors if the field doesn’t exist. If this is allowed, then at least it introduces an inconsistency between normal code and guards.

[josevalim]

@axelson

I guess I’ll voice a dissenting opinion. If we can’t support all of pattern matching (with the existing semantics) in defguard/1 then I think we shouldn’t allow it at all.

This would be a concern of mine if it was a silent failure but we can raise a very good error message here. Plus moving a binary match to a guard would probably have the largest performance implementation of them all due to the match context optimizations.

[fertapric]

@josevalim

. However, given that we have Map.fetch!/2 (equivalent to map_get/2 ) and Map.has_key?/2 (equivalent to is_map_key/2 ), adding those two new guards means we would break those rules, as we simply can’t remove the Map functions.

If those functions were added to Kernel, would the functions Map.fetch!/2 and Map.has_key?/2 be deprecated?

[Eiji]

@fertapric

If those functions were added to Kernel, would the functions Map.fetch!/2 and Map.has_key?/2 be deprecated?

I believe that it will not happen soon (if any). Hard remove of such functions would be not sooner than in 2.0.0 version (notice semantic versioning).

[ConnorRigby]

FWIW, i personally have some bad habits of making variables such as __x__ in my code for some shady macros. That’s not to say i’m correct or incorrect doing this, but it is a pretty aggressive change.

I also don’t think it would be much of an issue since i think the compiler has some built in mechanisms for dealing with this.

[namelos]

I totally understand why people downvote #3.

But, just I found my def lines are often longer than my implementations.

defguard is great, though people will keep trying stuff like user.age >= 21 and then learn.

And I believe user.age >= 21 is one of the most obvious or natural way people can come up with their instinct. As for defguard is something more people need to read more documentation to solve their own problem.

Instinct is a better documentation for people while adopting language.
When a C* programmer use another C* language they assume a lot of things, so they can confidently learn fast.

As for #3 itslef, I find user.age is about reaching down properties in a type (mentally similar to pattern) as apposed to more dynamic use cases like call a function, actually I’m pretty fine with FunctionClauseError for #3.

[taiansu]

1. Yes. If the function duplication is the main concern, then I’ll prefer Kernel.is_map_key/2 and Kernel.map_get/2 sit along with Kernel.elem/2 and Kernel.hd/1, then alias and/or soft deprecate the functions in Map module.

2. Yes. For me that defguard/2 doesn’t support pattern matching is inconsistent-ish. It’s one more rule to remember that defguard is the only one can’t pattern matching, compare to all the other def alike things.

3. Mixed feeling. To have map.field in guards is very intuitive and convenient. And I’m almost pretty sure that I’ll use it a lot if we have it. But for me, it’s like one more rule to remember. And I think we lose one excellent opportunity to educate newcomers that pattern matching can do a lot.

Matthew E. May said:

Something is elegant if it is two things at once: unusually simple and surprisingly powerful.

To me, pattern matching is in that category.

[English3000]

Here’s my 3 cents:

1. no for map_get/2, yes for is_map_key/2

As a newer user of Elixir (since v1.6), my experience is that keeping data structure-specific methods in the Kernel module is extremely counter-intuitive. For example, I did not even know map_size/1 existed (I’d move it to Map.size/1). Also, length/1 is in there yet it ONLY works on lists (hence, I think this should be converted to List.length/1, just like there’s presently a String.length/1).

Of course, this principle should not be overextended. + absolutely belongs in the Kernel, even if it only works for numbers.

I think the domain of the Kernel should be for constructs related to metaprogramming, e.g. def, and syntactic sugar, e.g. +.

This will meaningfully reduce the size of the Kernel (which should yield slight performance benefits, right?) and make the data structure-specific functionality currently housed there much more user-friendly… because if you don’t see the functionality in its data structure’s module, the natural assumption is that it just hasn’t been implemented (e.g. Tuple has no access method! Why is elem/2, which ONLY works on tuples, in the Kernel?).

As applied to the two proposed methods:

• map_get/2 should NOT be added because it is redundant (already exists as Map.get/2) and data structure-specific (only works predictably on maps, as keyword lists may have duplicate keys)

• is_key/2 (the name I’d use, as this functionality can be used not only for a map but also for a keyword list) is metaprogramming-related and belongs in the Kernel

2. yes

@josevalim

If the main goal of map_get/2 and is_map_key/2 is to help in the construction of composable guards, then one possible route to take is to expand the defguard construct, introduced in Elixir v1.6, to also support patterns.

@michalmuskala

No, we changed the name of a couple of them. elem and put_elem for example. We can’t adopt the Erlang names for map because our semantics for get mean returning nil if a key is not found. The “correct” name would be map_fetch!/2 . We also need to swap the argument order.

There are even more changes - especially in operators, where almost all have different names. The most notable examples being andalso is and and orelse is or , while and and or don’t exist in Elixir at all. I don’t think “keeping close to erlang” should be a factor here.

3. not yet…

@dimitarvp

Absolutely not. Many people come to Elixir from Ruby and Javascript, and they just expect certain syntax constructs to just work everywhere; they will be very disappointed when such a mechanism has only partial application and will probably leave.

@dimitarvp

In my eyes, Elixir (and parts of modern JS) brought functional programming to the masses and the community should do its very best to keep it minimal yet very capable and thus average-programmer-friendly.

Maybe this third discussion should be resurrected in the future, if/when evolutionary changes in Erlang or Elixir make it relevant and feasible again, and without the compromises several people already outlined.

@michalmuskala

today there’s difference in how code fails inside and outside guards (e.g. hd([]) might fail with an exception or just fail the current guard clause). When the expression fails or succeeds is always the same, though - this would break it and break it completely silently.

[yurko]

@English3000

As a newer user of Elixir (since v1.6) , my experience is that keeping data structure-specific methods in the Kernel module is extremely counter-intuitive. For example, I did not even know map_size/1 existed (I’d move it to Map.size/1 ). Also, length/1 is in there yet it ONLY works on lists (hence, I think this should be converted to List.length/1 , just like there’s presently a String.length/1 ).

That’s a good point, moving functions around would not be backwards compatible, but I think the documentation of related modules must mention kernel functions (or have their aliases, defdelegate or something).

[English3000]

This may be a little controversial, but if it were me, I’d copy the functionality over to the data structures’ modules, treat the ones in the Kernel as deprecated and eventually remove the documentation so new users wouldn’t use them–all while keeping them available in the Kernel to allow for backwards compatibility.

Of course, I doubt this would be done, so I’d be interested in hearing why/what the thinking was behind putting the data structure-specific methods in the Kernel instead of their data structure’s module.

Hmm… actually it could very easily be done via defdelegate!

[fertapric]

@English3000

This may be a little controversial, but if it were me, I’d copy the functionality over to the data structures’ modules, treat the ones in the Kernel as deprecated and eventually remove the documentation so new users wouldn’t use them–all while keeping them available in the Kernel to allow for backwards compatibility.

Some of those functions are used in guards, so they are a bit special. I think moving those to different modules would be confusing because only certain functions of certains modules could be used in guards. Imagine:

def foo(map) when Map.size(map) > 2 do
  ...
end

Looking at that code, it wouldn’t be weird to think that this code might work:

def foo(map) when Map.has_key?(map, :a) do
  ...
end

or any other function of the Map module.