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

GDScript: unpacking of arrays using matcher-like syntax #15561

Closed
wants to merge 1 commit into from

Conversation

poke1024
Copy link
Contributor

A variant of #15216 using the syntax proposal by @letheed. Examples:

	var a
	var b
	[a, b] = [1, 2]
	print(b)

and

func foo():
    return ["one", "two"]

func _ready():
	var [a, b] = foo()
	print(b)	

and

func _ready():
	for [x, y] in [[1, "a"], [2, "b"]]:
		print(x)

Looking at recent programming language development, it looks to me that while the magic unpacking known from Python and Lua certainly is well established, a lot of newer languages like Apple's Swift, nim, Rust and Scale deliberately make unpacking explicit like in this PR (see https://rosettacode.org/wiki/Return_multiple_values). I like it, I think it's very in tune with gdscript's pattern matching.

@karroffel
Copy link
Contributor

This looks cool!

I think it could be easier by making non-trivial assignments (so everything except var x = exp) use a match statement internally.

This could be done by simply adding in syntactic sugar. For example

From this

func _ready():
	var [a, b] = foo()
	print(b)	

the following code could be generated (or different versions of it):

func _ready():

	# var [a, b] = foo()
	var a = null
	var b = null
	match foo():
		[var a', var b']:
			a = a'
			b = b'
		_:
			# the implementation could report or deal with the mismatch somehow
			DEBUG_WARN("pattern matching assignment failed in file XXX at like XXX")
	print(b)

Currently GDScript lacks a form of error/exception handling. With such a system in place it could be possible to cleanly deal with mismatched patterns, otherwise it could just print a debug warning for now.

@poke1024 poke1024 mentioned this pull request Jan 26, 2018
@akien-mga
Copy link
Member

Updated: We've discussed this today on IRC, and this feature proposal is pre-approved for Godot 3.1. The code itself still needs to be reviewed and the implementation tested, but the design and feature seem good.

@reduz
Copy link
Member

reduz commented Jun 20, 2018

Regarding code review. I think this behavior is much easier to do within the parser, and not touching the compiler. Just add extra statements.

@LikeLakers2
Copy link
Contributor

LikeLakers2 commented Jun 20, 2018

My apologies if I missed something, but I'm not sure I understand [a, b] = [1, 2] over something like a, b = [1, 2]. The second one seems a bit more natural to type.

@vnen
Copy link
Member

vnen commented Jun 20, 2018

My apologies if I missed something, but I'm not sure I understand [a, b] = [1, 2] over something like a, b = [1, 2]. The second one seems a bit more natural to type.

I personally find the first much more natural, because the structure is the same. So [a, b] correlates to [1 , 2], you can easily see how one fits into the other.

This could lead also to match-like syntax to ignore some of the results, like: [a, _, b] = [1, 2, 3].

@LikeLakers2
Copy link
Contributor

LikeLakers2 commented Jun 21, 2018

I personally find the first much more natural, because the structure is the same. So [a, b] correlates to [1 , 2], you can easily see how one fits into the other.

I blame my experience in Ruby for wanting a, b = [1, 2] over [a, b] = [1, 2].

This could lead also to match-like syntax to ignore some of the results, like: [a, _, b] = [1, 2, 3].

I'd have to ask what happens if we did something like [a, b] = [1, 2, 3] then. In Ruby, a would get assigned 1, and b would get assigned 2, with the 3 being ignored. How would we handle that?

Also, what about [a, b, c] = [1, 2]? In Ruby, a and b would be assigned their appropriate values, while c would be assigned null (I checked, it assigns null in this case, not just leaves it alone). Are we handling it the same here?

@vnen
Copy link
Member

vnen commented Jun 21, 2018

I'd have to ask what happens if we did something like [a, b] = [1, 2, 3] then. In Ruby, a would get assigned 1, and b would get assigned 2, with the 3 being ignored. How would we handle that?

The same way, first values are assigned, rest is ignored.

Also, what about [a, b, c] = [1, 2]?

That's more complex. I'm not sure if assigning null is the best, or if raising an error is more appropriate. There might be something wrong if it happens, OTOH you may have a function that returns arrays in different sizes and then you can check if c == null to know whether it was really set.

@LikeLakers2
Copy link
Contributor

I'm not sure if assigning null is the best, or if raising an error is more appropriate.

I feel like assigning null (or alternatively, just not setting the extra variables at all) would be better, because then it provides consistent functionality on both sides. If there's more results than variables, the excess will be ignored; and if there's more variables than results, the excess will be ignored (or null'd). It would also enable people to add more to the return value of a function without having to go through and ensure there's an _ everywhere in existing usages, which in turn makes it easier to maintain compatibility.

@letheed
Copy link
Contributor

letheed commented Jun 21, 2018

@LikeLakers2 I disagree that it makes things easier to maintain. It makes things easier to write, but harder to maintain.

If I wanted the first two elements of an array of 2 or more elements, I'd expect to write something like var [a, b, ..] = [1, 2, 3, 4, 5] from the start. And I would expect var [a, b] = [1, 2] to produce an error if the rhs wasn't a array of length 2 exactly. To me this is the same thing as matching, and therefore should have similar semantics and syntax. If you're not sure that the rhs is going to be of length 2 then you should probably be using a match statement.

GDScript seems more designed for ease of writing rather than maintaining though so I understand the choices being made. I guess this is part of the more functional vs. more imperative choice for GDScript's future.

@OvermindDL1
Copy link

OvermindDL1 commented Jun 21, 2018

Honestly, if a match syntax is going to be used then it should function in Elixir as it is explicit and less surprising than getting things like random nulls and such, examples:

iex(1)> # These all work
nil
iex(2)> {a, b} = {1, 2} # Tuples (if added to godot)
{1, 2}
iex(3)> a
1
iex(4)> b
2
iex(5)> [a, b] = [1, 2] # Lists
[1, 2]
iex(6)> [a, b | rest] = [1, 2, 3, 4, 5, 6] # List with taking the rest into a variable
[1, 2, 3, 4, 5, 6]
iex(7)> a = 42 # Normal assignment in elixir is just a matcher as well
42
iex(8)> {^a, b} = {42, 2} # Pinning (^) a value enforces that it must match
{42, 2}
iex(9)> 
nil
iex(10)> # These all fail
nil
iex(11)> {a, b} = {1, 2, 3} # Mismatched tuple size
** (MatchError) no match of right hand side value: {1, 2, 3}

iex(11)> [a, b] = [1, 2, 3] # Mismatched list size
** (MatchError) no match of right hand side value: [1, 2, 3]

iex(11)> {a, b} = [1, 2] # Mismatched types
** (MatchError) no match of right hand side value: [1, 2]

iex(11)> 
nil
iex(12)> # You can ignore values:
nil
iex(13)> [_, b | _] = [1, 2, 3, 4, 5]
[1, 2, 3, 4, 5]
iex(14)> b
2
iex(15)> 
nil
iex(16)> # You can match as deep in a structure as you want:        
nil
iex(17)> {_, [a, b], _} = {1, [2, 3], 4}
{1, [2, 3], 4}
iex(18)> a
2
iex(19)> b
3
iex(20)> # You can perform multiple matches at once in source order:
nil
iex(21)> case [1, 2, {3, 4}, 5, 6] do
...(21)>   {_, b} -> b
...(21)>   [_, b] -> b
...(21)>   [_, _, {_, d} | _] -> d
...(21)>   _ -> 0 # fallback since `_` matches anything
...(21)> end
4
iex(22)> # You can add 'guards' to constrain matches as well:
nil
iex(23)> case 42 do    
...(23)>   i when i < 0 -> 0
...(23)>   i when is_integer(i) or is_float(i) -> i
...(23)>   _ -> throw :not_an_integer
...(23)> end
42

I.E. you can use _ to ignore and not create a binding (things like _blah also does not create a binding variable but lets you 'document' it regardless, which is great for readability unlike the above examples), you can match 'deep inside a structure (Elixir also has dictionaries in addition to tuples, lists, integers, floats, atoms, strings, and a few others), the matchers can have guards (a restricted set of functions specifically designed for very fast generated code thus keeping matchers very fast, things like simple type tests, value comparisons, a few other things) with specific match semantics, among more. Failed matches (either at top level = or no matching case in a case) will throw an exception. It is an exceedingly powerful and very simple to use and efficient system.

Matchers make for great built-in asserts for pre/post conditions as well.

@OvermindDL1
Copy link

And some docs and articles on it, how it works, and why it is amazingly useful:
https://elixir-lang.org/getting-started/pattern-matching.html
https://elixir-lang.org/getting-started/case-cond-and-if.html
https://elixirschool.com/en/lessons/basics/pattern-matching/
https://blog.carbonfive.com/2017/10/19/pattern-matching-in-elixir-five-things-to-remember/
https://joyofelixir.com/6-pattern-matching/
https://revelry.co/pattern-matching-elixir/

And yes, this matching falls into the functions as well, you can define a function like:

def blah(i) when is_integer(i), do: to_string(i)
def blah({_, i}), do: blah(i)
def blah(%{"bloop" => i}) when i >= 0, do: to_string(i)
def blah(s) when is_binary(s), do: s
def blah(unknown), do: throw {:unhandled, unknown}

The different 'heads' of a function must be adjacent (can't put different functions are arities between other heads), and it essentially just compiles into:

def blah(value) do
  case value do
    i when is_integer(i) -> to_string(i)
    {_, i} -> blah(i)
    %{"bloop" => i} when i >= 0 -> to_string(i)
    s when is_binary(s) -> s
    unknown -> throw {:unhandled, unknown}
  end
end

So that might be another interesting feature to swipe.

(As an aside, I personally dislike Elixir's syntax, very ruby'ish, and prefer erlang's syntax, elixir is built on top of and compiles to erlang, but of course the actual gdscript code should be made gdscript'y syntax regardless.)

@letheed
Copy link
Contributor

letheed commented Jun 21, 2018

I would have preferred actual tuples, rather than arrays. But I'm not sure the complexity is worth it for GDScript.

@akien-mga
Copy link
Member

This will need to be rebased now that typed GDScript has been merged.

@poke1024
Copy link
Contributor Author

A basic rebase, needs more work.

@akien-mga
Copy link
Member

Since we're reaching feature freeze for 3.1 and some more work is needed here, moving to the 3.2 milestone (which incidentally has many GDScript features planned, so this would go well with it).

@akien-mga akien-mga modified the milestones: 3.1, 3.2 Aug 23, 2018
@akien-mga akien-mga removed the request for review from reduz April 30, 2019 15:26
@akien-mga
Copy link
Member

A basic rebase, needs more work.

What additional work is required to have this merge ready?

There wasn't as much GDScript focus as expected during 3.2 development as @vnen was quite busy, but this should hopefully be something that we could merge for 4.0 once 3.2 is released in a couple of month (we're about to enter feature freeze for 3.2 this week, so now might be a bit late to include the feature if it's not merge ready yet).

@akien-mga akien-mga modified the milestones: 3.2, 4.0 Aug 28, 2019
@aaronfranke aaronfranke marked this pull request as draft April 21, 2020 22:25
@aaronfranke
Copy link
Member

@poke1024 Is this still desired? If so, it needs to be rebased on the latest master branch.

If not, abandoned pull requests will be closed in the future as announced here.

@vnen
Copy link
Member

vnen commented May 14, 2020

At this point is better to not rebase this. I'm rewriting the GDScript implementation so this code will have to be redone anyway. Eventually I'll add this feature too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants