godot
godot copied to clipboard
godotengine
Treat Variant as safe where it is explicitly expected
Closes #69068
Summary for current state:
static func variant() -> Variant: return null
var member_i = variant() # safe
var member_e: Variant = variant() # not safe, bad
func param_i( param = variant() ) -> void: pass # safe
func param_e( param: Variant = variant() ) -> void: pass # not safe, bad
func return_i(): return variant() # safe
func return_e() -> Variant: return variant() # not safe, bad
func check() -> void:
var var_i = variant() # safe
var var_e: Variant = variant() # safe, good
var_i = variant() # not safe
var_e = variant() # not safe, bad
param_i( var_e ) # not safe
param_e( var_e ) # not safe, bad
Three first cases - member, param and return - show that explicitly typing something as Variant is punished, which is clearly wrong.
Variable initialization is already safe (thanks to #58185), but then it is more strange that an assignment is not (when it comes to explicitly typed Variant variable).
Last one - about passing Variant argument to explicitly Variant parameter - should be safe too in a good type system, unsafe lines are those that are inside a function when and if a parameter ends up being casted to something.
All of this reduces number of noise when it comes to unsafe type lines.
Marked is, == and != operators as safe for usage with variants and typed them as booleans. Currently:
if var_e == null: pass # not safe, bad
if var_e != null: pass # not safe, bad
if var_e is Node: pass # not safe, bad
Hm... so comparing two Variants is unsafe, have not expected that. Limited blank safety only to comparing to null.
Since arrays and now dictionaries are compared by contents, usual comparison may include a conversion and also does not work between two arbitrary types, starting to think that something like === from js might make sense here - operator that checks that types are equal and compares inputs only by reference if possible, so no conversions, no contents checks and safe for all type combinations. Or at least a global helper method is_equal_strict for that.
Updated PR to allow inference of hard Variants for variables:
var var_i := variant() # error -> safe
Removed some inconsistencies between resolving member variables and local ones.
Added a test.
Just went through the PR. Didn't really do a deep dive, but all the code changes make sense to me, as well as the conceptual changes to what is safe and isn't safe :)
Just had a question above for something that I noticed afterward was already this way, so I guess no harm in leaving it this way either :)
Updated PR to allow inference of hard Variants for variables:
var var_i := variant() # error -> safe
I'm not sure if this is a good idea. Generally one uses type inference expecting to become of that specific type. Since Variant is not really an specific type, it should not be valid for inference, even if the original value is hard typed to be Variant. I fear it's quite easy to do this mistakenly by accident.
In this case it would be better to either not use static type at all or explicitly type it as Variant.
var var_i := variant() was part of unification, currently in master.
I consider the line itself as type safe. Unsafe lines would be those where var_i is used as anything other than variant. So if var_i is used just as a dictionary key, that's pretty type safe. (After type parameters for dictionaries are introduced one will be able to communicate if they really expect variants as key/value or not and those lines might become type unsafe.)
not use static type
In my view there is a difference between hard variant type and weak variant (untyped) type. With hard variant it should be safe to both read and write variant while with weak one you are told that the value is of some unspecified type, you can expect anything but it does not mean that you can safely set it to anything, so:
var a := variant()
a = another_variant() # type safe
var b = variant()
b = another_variant() # type unsafe
And it is also strange to punish properly typing things as there is no other way to type "anything" other than with Variant.
Also important to highlight that I'm talking about type safety, not simply safety, even type safe code can be wrong and unsafe. This is strictly about types, so maybe a = another_variant() is not actually safe, but from perspective of types it should be.
