-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Extend match
syntax to enable safe type narrowing
#6941
Comments
This is also useful with respect to godotengine/godot#76843, because narrowing a nullable type (e.g. |
I feel like each option has a different strength:
So... why not introduce both? Type-checking pattern with |
Edit: I would actually like option B much more if we just left out the Looking over the options again, I think The match syntax in GDScript is match expression:
pattern:
# block One of the nice things about the available match patterns in GDScript is that for the purposes of understanding code, they can all be read aloud as essentially
Even patterns like With that in mind, I read option A's match expression:
var new_var as String: as match expression:
is String doesn't read the same way because |
I am now in favour of option B (with a modification), after reading the comments left here and giving the issue much more consideration. I initially advocated for option A in the belief that it would be more consistent, as it aligns more with the way we declare variables. As shown below, a syntax which requires prepending an arbitrary variable name confuses the overall semantics, and makes it difficult to see at a glance what is being checked against. I am including potential future extensions of the syntax. In introducing this significant new feature, it is important that we consider how future adaptations would interact with it. match foo:
as int:
pass
var vector3 as Vector3: # Difficult to compare; offset; add/remove var name from the start
pass
as Node2D:
pass
var some_node as MeshInstance3D | CollisionShape3D | PhysicsBody3D: # Possible future syntax
pass
var node as Node3D:
pass
as Control:
pass
var vector2 as Vector2:
pass
var str = "string which is long and easy to mistype": # Possible future syntax
pass # Messy. We are now comparing values, but it is difficult to tell the difference.
"a string":
pass
var attitude = "hostile", "neutral", "friendly": # Possible future syntax
pass
"A", "B":
pass The below variation, in comparison, is beautiful and readable - in the mode of GDScript. match foo:
is int:
pass
is Vector3 var vector3:
pass
is Node2D:
pass
is MeshInstance3D | CollisionShape3D | PhysicsBody3D var some_node: # Possible future syntax
pass
is Node3D var node:
pass
is Control:
pass
is Vector2:
pass
"string which is long and easy to mistype" var str: # Possible future syntax
pass
"a string":
pass
"hostile", "neutral", "friendly" var attitude: # Possible future syntax
pass
"A", "B":
pass Crucially, the case statements begin with the indicator of the checking operation. Instead of having the analyser/interpreter look past a match foo:
var bar█ # Is it a type check? A value check? A fallthrough? Who knows... match foo:
var b█is Vector2: # The user goes to the line start to add/remove a var, confusing the analyser The fact is, the With You may notice that I omitted
You could also view each case as having an implicit operator at the beginning. This makes match foo:
== "a string": # Implied by lack of anything else
pass
is String: # Explicit
pass
dict_matches { "key": "value" }: # Implied by {
pass
array_matches [ "item", .. ]: # Implied by [
pass |
Agreed. I like the way you wrote out the operator implied by the string, dict, and array example above.
You lost me here. If all other match patterns start with an implied operator, why should this new pattern be the only one that starts with an explicit operator? That's essentially my complaint about starting with |
It's about existing language semantics. All of the other cases use an implied equality/match operator ( |
If the choice is between starting the line with |
I'd like to bring up an alternative that @SlugFiller, @ator-dev, and I discussed recently on RocketChat. Instead of extending For example, consider a new expression of the form x is var new_var : Type This expression returns the result of var new_var : Type
if x is Type:
new_var = x
x is Type Here's an example of how this expression could enable safe type narrowing. func _input(event: InputEvent) -> void:
if event is var event_mouse : InputEventMouse :
pass # react to mouse using "event_mouse" local variable
else if event is var event_key : InputEventKey :
pass # react to keyboard using "event_key" local variable
else:
pass # react to any other event using "event" parameter Alternatively, using func _input(event: InputEvent) -> void:
match event:
var event_mouse : InputEventMouse : # implied expression is "event is var event_mouse : InputEventMouse"
pass # react to mouse using "event_mouse" local variable
var event_key : InputEventKey : # implied expression is "event is var event_key : InputEventKey"
pass # react to keyboard using "event_key" local variable
var event_other # the usual fall through pattern can now be understood as "event is var event_other : Variant"
pass # react to any other event using "event_other" local variable As you can see, this approach helps justify the current match fall through pattern, whose syntax I find somewhat funny compared to the other match patterns. match x:
var y: # Why is this the pattern for fall through? It can now be understood as "x is var y : Variant", which is always true and always binds "x" to "y" One important objection that was mentioned was the danger of relying on default/uninitialized values for the cases where Specific syntax aside, the core idea is to solve explicit narrowing by introducing a new expression. I think this approach has a few merits:
|
I should have mentioned, this might be better in a separate proposal, even though it does relate to this one. It's quite a complicated topic that would take effect all over the language. I recommend copying it over if you want more specific discussion. (: I will do my best at answering these points though.
The thing is, I don't think the proposed change to
This needs a separate discussion I think. I'm not convinced, because I don't think we have any expressions that are anything more than a series of operations that equates to an in-place value. Scoping of this would be difficult.
Perhaps, but I also see value in introducing specific syntax to Edit: Actually I acknowledge that the |
I tend to agree that an expression that atomically both returns a boolean and declares a variable is the way to go.
I think it would be better to use if event is var event_mouse as InputEventMouse:
pass # event_mouse can be used here Or better, copy C# syntax, which is clear and concise: if event is InputEventMouse event_mouse:
pass # event_mouse can be used here |
I believe you don't have to rely on uninitialized values, because you'd statically know that on that branch the variable would not "exist", and you should not be able to even reference it. For example, it's important to allow this pattern: if not x is int i:
# print(i) # would be a static compiler error, as i is undefined in this branch
return null # useful when I KNOW Variant x is int and don't want more unnecessary nesting
my_int_fn(i) That's how it works in C#, btw - you can even add compound logic with if a and x is int i:
print(2*i) # i is ALWAYS initialized here
else:
return # i is NEVER initialized here, so not accessible if a or not x is int i:
return # i is NEVER initialized here, so not accessible
# i is ALWAYS initialized here if a or x is int i:
print(2*i) # ⛔ should give compiler error because i is NOT guaranteed to be initialized here
fn()
else:
return # i is NEVER initialized here, so not accessible # a working refactoring of the example above
if a or x is int: # no variable declaration
if x is int i: # only introduce it separately
print(2*i) # i is ALWAYS initialized here
fn() # i is NOT accessible here, since it's completely out of scope
else:
return # i is NOT accessible here, since it's completely out of scope
# alternative valid refactoring
if a or x is int: # no variable declaration
print(2*i if x is int i else null) # type-safe since print expects Variant anyway
fn() # i is NOT accessible here, since it's completely out of scope
else:
return # i is NOT accessible here, since it's completely out of scope |
After godotengine/godot#80247 (#632), I remembered our discussion in Godot Contributors Chat and and the following syntax began to look more consistent to me (I just remembered if we have other declarations in which we cannot specify the type): match foo:
# Optional type specifier for match bind pattern.
var bar: String:
# ...Actions with bar... Although in this case the type specifier is for filtering, it is different from But actually I'm not sure if we should use match to narrow the types. In my opinion the following current solutions are acceptable: var foo: Node
if foo is Node2D:
var bar: Node2D = foo
# Actions with `bar`... var foo: Node
var bar := foo as Node2D
# If `foo` is not `Node2D`, then `bar` equals `null`.
if bar:
# Actions with `bar`... As for nullable types, in my opinion they are not much different from other types ( var foo: Variant
var bar := foo as String?
# If `foo` is not `String?`, then `bar` equals `null` (default of `String?`).
if bar:
# Actions with `bar`... var foo: Variant
var bar := foo as String
# If `foo` is not `String`, then `bar` equals `""` (default of `String`).
if bar:
# Actions with `bar`... The disadvantage of these options is that you cannot specify a different default value in case of an error, or check if there was an error (in the case of var foo: Variant # Or `String?`. Doesn't matter, any `String` supertype.
if foo is String:
var bar: String = foo
# Actions with `bar`...
else:
# Error processing... var foo: Variant # Or `String?`.
var bar: String = "default"
if foo is String:
bar = foo
# Actions with `bar`... |
I don't like the idea of splitting type narrowing across two lines. I have several reasons for this, but perhaps the biggest one is that inserting a line in between would invalidate the typing. For example: if foo is Node2D:
var bar : Node2D = foo ensures that if foo is Node2D:
if randf() > 0.5 : foo = 0
var bar : Node2D = foo doesn't work half the time. In principle, I suppose one could statically analyze the intervening code to determine whether all paths result in a valid |
I think the solution for type narrowing should play nicely with match pattern guards if those are ever implemented. In fact, type narrowing can be seen as a special case of pattern guards. For example, if GDScript had pattern guards such as match x:
var y if x > 0:
# code treating y as positive then a natural way to do type narrowing would be match x:
var y : int if typeof(x) == TYPE_INT and x > 0:
# code treating y as a positive int Arguably, the |
Describe the project you are working on
GDScript
Describe the problem or limitation you are having in your project
In the current implementation of GDScript, there is a limitation when it comes to type narrowing. Developers are unable to treat a variable as a more specific type within certain code blocks, such as if statements or match expressions. This limitation can lead to verbose and less readable code.
The safety of a type check is highly contextual. In segment A,
foo += 1
would seem to be what it is: an entirely safe operation. However, something as simple as adding a function call in B might violate that assumption. Perhaps it makesfoo
a string, meaning that the operation will no longer succeed. C transparently violates the assumption, while D is more nebulous; during the 1 second wait period, anything could modifyfoo
to be something else. The point being that it is not sensible to try to infer type safety based on an earlier check. Adding special cases such as assuming type safety directly after the check (E) is a workaround, but one that significantly increases the complexity of the analyser and is arbitrary.See "If this enhancement will not be used often ..." for workaround details.
A
B
C
D
E
Describe the feature / enhancement and how it helps to overcome the problem or limitation
Discussed on Rocket.Chat with several contributors. Hopefully the discussion will be enhanced here.
We can extend the already exceptional
match
syntax to complete type checking.Case syntax should be introduced where:
as
would be successful (i.e. not returnnull
for nullable types), the case block is executed.And theoretically:
"A", "B", "C":
case syntax already allowed. We could use commas in the same way, or pipes (|
) to conform with common type theory syntax. This might amount to pretending we have a feature which we don't (yet), however, and couldn't be combined with an explicit variable since it would not be type safe in current GDScript.Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
Option A
Option B
If used currently in GDScript with proposed syntax (option A):
If this enhancement will not be used often, can it be worked around with a few lines of script?
Due to the demonstrated type safety limitations with type checking, in order to enforce type safety, developers must manually cast variables a lot of the time. The resulting code is less maintainable and readable, due to being more verbose and specifying types manually; essentially a violation of DRY (don't repeat yourself).
Is there a reason why this should be core and not an add-on in the asset library?
This is core GDScript.
The text was updated successfully, but these errors were encountered: