-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Provide defguard #2469
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
Comments
I love this future feature but I think that |
Even though the scope of what you can do with |
Yeah, On Tuesday, July 1, 2014, Eric Meadows-Jönsson [email protected]
|
I did something like this some time ago. Quite out of date now, and it does no work to enforce the limitations of guards. However, I found using |
My vote goes for do/end + friendly docs that says keep it simple. |
|
Somehow this looks like the most natural choice to me:
Elixir has body-less functions. This one extends the idea a bit to associate a guard expression (it is obvious here that only things allowed in guards expressions are also allowed in Using
This doesn't look Elixirsh:
It could instead be
to look somewhat similar to how |
Body-less functions have no logic in elixir, so we shouldnt introduce a type of body-less function that has logic. Additionally, with Seeing a Use |
+1 one on the |
Yup, reading through these comments I'm inclined to prefer the |
|
I tried to prototype defmacro my_guard(arg) do
arg = operation_on_arg(arg)
quote do
unquote(arg)
end
end That is, doing something in the macro before entering the |
@whatyouhide that's a very, very good point. Maybe, instead of providing defguard as a Kernel macro, we could provide |
@josevalim so, would we write something like: defmacro ends_in?(n, digit_or_digits) do
digits = List.wrap(digit_or_digits)
Macro.to_guard do
rem(n, 10) == digits
end
end I'm not sure if I got this right :\ |
More like this: defmacro ends_in?(n, digit_or_digits) do
digits = List.wrap(digit_or_digits)
expr = quote do: rem(n, 10) == digits
vars = [digits: digits]
Macro.to_guard(expr, vars, __CALLER__)
end |
I'm not sure why |
It is a bug, both should be there. :) |
I am stepping back on this one. |
Reopening given the mailing list discussion. @whatyouhide if someone needs to process the arguments before quoting, then they shouldn't use defguard. :) We will also go with the third option:
It is the one that makes most sense because the right side of when needs to be guard expressions. |
So, to reify the spec on this, you want:
I still think there are some problems with the |
@christhekeele yes. And it should also raise if a non-valid guard expression is used in |
Here's my issue with the def foo(bar) when is_list bar
def foo(bar) when is_map bar Parallel syntax suggests you could do the following: defguard foo(bar) when is_list bar
defguard foo(bar) when is_map bar Of course that makes no sense in this context. The implementation discussed above would generate macros that warn The defguard foo(bar) do
is_list(bar) or is_map(bar)
end Admittedly, if the guard writer does provide guard clauses things get a little weird: defguard foo(bar) when is_list bar do
hd(bar) == :baz
end If that's translated using defmacro foo(bar) when is_list bar do
case Macro.Env.in_guard?(__CALLER__) do
true -> quote do
hd(unquote(bar)) == :bar
end
false -> quote do
r = unquote(bar)
hd(bar) == :bar
end
end
end This will never work inside guards because I can't think of a good way to support guards in defguards at the macro level, perhaps someone really clever could figure it out though. Failing a proposal for that, though, TL;DR: I think the |
This naive one-clause addtion to It whitelists the allowed local calls documented here, permits simple variable references, and doesn't allow blocks, remote calls, function definition, assignment, or pattern matching. |
@christhekeele a |
Just wanted to post some notes here. I took another deep dive into implementing this last night on top of edge 1.5.x and got a quite a bit further, but still ran into some blockers in isolating what's valid in guards. This is much trickier than I originally surmised and has really gotten me acquainted with Elixir's internal implementation, it's been a blast. 😃 I'm keeping a log of my investigation here. Documenting it as I go has helped me make many incremental gains over my last attempt, although I've spent at least much time on the writeup as the implementation. If you happen to read your way through it all, I'd appreciate any comments or insights! |
Idea dump: I'm curious how it would be to implement an def blah(some_struct) when is_struct(some_struct), do: :blah
def blorp(some_struct) when is_struct(some_struct, MyStruct), do: :blorp
def bleep(some_struct, dyn_struct_name) when is_struct(some_struct, dyn_struct_name), do: :bleep Then there should be some way to create an unnamed match as well, so perhaps something like this: defguard is_struct(%{__struct__: struct_name}) when is_atom(struct_name) # Maybe some other checks?
defguard is_struct(%{__struct__: struct_name}, struct_name) when is_atom(struct_name) Could cause the above first 3 examples of def blah(some_struct) when is_struct(some_struct), do: :blah
# Desugars into:
def blah(some_struct = %{__struct__: struct_name_guardmatch_0}) when is_atom(struct_name_guardmatch_0), do: :blah
def blorp(some_struct) when is_struct(some_struct, MyStruct), do: :blorp
# Desugars into:
def blorp(some_struct = %{__struct__: struct_name_guardmatch_0}) when is_atom(struct_name_guardmatch_0) and (struct_name_guardmatch_0 === MyStruct), do: :blorp
def bleep(some_struct, dyn_struct_name) when is_struct(some_struct, dyn_struct_name), do: :bleep
# Desugars into:
def bleep(some_struct = %{__struct__: struct_name_guardmatch_0}, dyn_struct_name) when is_atom(struct_name_guardmatch_0 and (struct_name_guardmatch_0 === dyn_struct_name), do: bleep
Or something like that. It is definitely not a normal macro, very special, but this would give the most power. When 'def' walks over the defguard calls it just has a counter it increments for every usage to make unique variable names and so forth. Even calling |
@OvermindDL1 It's not possible since guards can have boolean expressions. EDIT: Changed the example. |
@ericmj Yeah I'd thought about those when thinking of them before, would probably have to break up the function head into multiple heads and have them call back down to the base defined body. It is doable, but a pain, however it would make for a very powerful |
@OvermindDL1 It's not possible with |
@ericmj Correct, that's why I mentioned that |
And I just implemented that hack to see how the syntax works. ^.^ A shell session: blah@blah MINGW64 ~/projects/tmp/defguard
$ iex -S mix
Eshell V8.2 (abort with ^G)
Interactive Elixir (1.4.0) - press Ctrl+C to exit (type h() ENTER for help)
iex>defmodule StructEx do
...> import Defguard
...> defguard is_struct(%{__struct__: struct_name}) when is_atom(struct_name)
...> end
{:module, StructEx,
<<70, 79, 82, 49, 0, 0, 5, 252, 66, 69, 65, 77, 69, 120, 68, 99, 0, 0, 0, 155,
131, 104, 2, 100, 0, 14, 101, 108, 105, 120, 105, 114, 95, 100, 111, 99, 115,
95, 118, 49, 108, 0, 0, 0, 4, 104, 2, ...>>, {:is_struct, 1}}
iex> defmodule Testering do
...> use Defguard
...> import StructEx
...> def blah(any_struct) when is_struct(any_struct), do: any_struct
...> def blah(_), do: nil
...> end
warning: unused import StructEx
{ iex:4
:module
, Testering,
<<70, 79, 82, 49, 0, 0, 5, 24, 66, 69, 65, 77, 69, 120, 68, 99, 0, 0, 0, 151,
131, 104, 2, 100, 0, 14, 101, 108, 105, 120, 105, 114, 95, 100, 111, 99, 115,
95, 118, 49, 108, 0, 0, 0, 4, 104, 2, ...>>, {:blah, 1}}
iex> Testering.blah(%{__struct__: Blah})
%{__struct__: Blah}
iex> Testering.blah(%{blah: 42})
nil
iex> Testering.blah(42)
nil
iex> As you can see, it works fine. ^.^ I'm also playing with the idea to have defguard accept an optional My hack is 'just' implemented enough for something this simple to work, would need work to have more, but considering it is less than 60 lines of code to do what I have now and adding more cases is easily expressed with how I have things split up, this could easily become a full |
@ericmj is correct. The only scenario where we would accept this trade-off is if someone did a careful study of the generated beam code under all of those circumstances and how BEAM will or won't optimize those patterns. |
What I'm doing is at each new I think that works well since if you do an |
The idea of keeping the `defguard foo when impl` formation, but allowing you to something for sophisticated use cases with a `do` block is pretty compelling. Not to implement features just for syntax's sake, though. I can't think of what you'd put there.
|
Yeah I could not come up with a good example either, though I just chalked that up to still having a bit of flu. ^.^ A |
How about add to defmodule Example do
defstruct [:sample]
defguardp is_not_empty_struct(%Example{sample: sample}) when sample != false
def prepare_struct(add_sample, struct),
do: if add_sample, do: Map.put(struct, :sample, "sample"), else: struct
def method_that_should_have_prepared_struct(struct) when is_not_empty_struct(struct),
do: # do something with struct ...
def method_that_should_check_struct(_, struct) with new_struct from &prepare_struct/2
when is_not_empty_struct(new_struct), do: # do something with struct ...
# or something similar ...
end
So before call Summary: function that when matching calls another function that determines if first function will match and returns extra prepared data, so we do not need to use |
Anything is possible, but like we have said multiple times, let's move this discussion to a more appropriate forum like the elixir core mailing list or the forum. This issue is about |
Folks, please stop derailing the discussion with other features. |
Closing in favor of the PR which is pretty much complete at this point. |
Just love this. |
Hey! guys, I was trying this feature with pattern matching and looks like it's not working as I expected or maybe I'm wrong with something basic: defmodule Data do
defguard is_capital(<<letter::binary-size(1), _ :: binary>>) when letter >= ?A and letter <= ?Z
def greet(name) when is_capital(name), do: "Hello, #{name}"
def greet(thing_name), do: "Hello, to #{thing_name}"
end The error is:
Is not let to use pattern matching with |
@manuel-rubio pattern matching is not supported by design. https://github.com/elixir-lang/elixir/blob/v1.6.6/lib/elixir/lib/kernel.ex#L4614L4625 |
Today implementing a guard is somehow complicated as we need to quote/unquote arguments depending if the function is being invoked inside or outside of a guard. Here is an example from the Record module:
The idea is to provide a
defguard(p)
macro that does all the work automatically and assert the expressions in the guard are valid. The example above would be rewritten as:defguard(p)
is extremely limited in scope:Since we have all limitations above, it is debatable if
defguard
should use thedo/end
syntax as it "promotes" code blocks. Here are other alternatives:or:
or even:
Thoughts?
The text was updated successfully, but these errors were encountered: