-
-
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
Make objects non-nullable by default #11054
Comments
I know it's much much easier said than done but... to be completely honest, I would hope for Godot 5 that the entire thing was redesigned from scratch (instead of this incremental patch-by-patch approach) so we could have a proper type system with everything. And only THEN adapt the engine around it to make proper use of it. In the order of priority below. Crucial missing features to fix forced Variant use in the engine (these require a deep redesign of the typing system):
Features to improve type-safety for the user (less priority, as less-optimized workarounds are usually possible):
This way we could finally "get rid of Variant" in the sense that we wouldn't ever have to deal with it unless we're really using reflection. E.g. we could have a project setting "no_reflection" to raise error if using Variant anywhere. |
Thanks for opening this issue. I've been thinking a bit about non-nullables and initialization. I think you're right that using
should not even be allowed to compile without a warning. I think it would be best for the compiler to identify a point after which all non-nullables variables must be initialized; the most logical point for this is after the call to However, I can already think of challenges for an approach like this. For one thing, attempting to ensure that all variables are initialized when
Any thoughts on these problems or other fatal flaws enforcing initialization in Also, while some of these are breaking changes, is this really true for all of them? It seems like some of them could probably be implemented without here. As some people mentioned in the discussion of issue 162, C# allows you to configure projects to disallow implicit nulls. Would that really be impossible in GDScript? I don't think people want perfect soundness, just the ability have the compiler flag dumb mistakes and inconsistencies. |
Even if this particular case can be validated by the static analyzer, in general we still cannot guarantee that the variable will not be accessed before initialization. For example, it could be in another function called in another variable's initializer (GDScript allows non-constant variable initializers). Or when resizing a typed array, as shown in the second example. Or for a million other reasons, see the halting problem and other fundamental computer science problems. Don't expect a compiler to prove Fermat's Theorem.
I am quite sure that most of what is described in this proposal cannot be implemented without major compatibility breakage. At least because it affects the core and its API. GDScript relies on and depends on core systems in many ways, we cannot solve this only on the GDScript side, it would require completely new layers of abstractions and runtime wrappers around the core. I am also skeptical about this kind of configurability. It complicates the model and implementation, increases the maintainability burden and the number of config-specific bugs. It would make third-party plugins harder to support, worsen their portability and compatibility. In my opinion, we should not complicate the already complex and full of technical debt part of the engine. |
I want to clarify a few things:
You say that "in general we still cannot guarantee that the variable [a variable?] will not be accessed before initialization." At the risk of saying something stupid, I can't figure out what you mean by this. What am I missing that would prevent the compiler from knowing that
will always be able to initialize all of the object's members and call
Your code snippet passes
I thought (apparently incorrectly) that you opened this proposal specifically to move that debate into its own thread, so when I suggested that not all of your proposed changes needed to break compatibility, I was really just referring to one part of the second "key point" you identify in your proposal. Specifically, I was trying to say that we could "disallow accepting null where an object type is expected" in GDScript without breaking compatibility. To my mind this change would have enormous benefits, even if the rest of your proposal was never implemented. Back in the #162 discussion, @geekley suggested four possible paths for adding null-safety to GDScript:
geekley’s view was that 3 is probably a non-starter, 4 was best, and 2 was bad but vastly preferable to 1. I agree with them; I think that holding this huge QoL improvement back until the engine's whole type system can be rewritten would be a shame. When you say you're skeptical of "this kind of configurability," I'm not sure if you're just saying that you're skeptical of adding null-safety to the whole engine and making it configurable system wide, or if you're saying that even limiting this configurability to GDScript would be a mistake. If you mean the former, then I agree completely. On the other hand, if you mean making object types null-safe by default should never be a configuration option, I agree with geekley that it would be better to add |
Well, half-right. It's right, except for the last part @unfallible.
So my current opinion on those approaches is a bit more like:
In general:
|
SSA can only be used for local variables, because they cannot be reassigned from the outside1. With class members, things are more complicated, they can (potentially) be changed by any call. Yes, we should add errors/warnings if the analyzer can reliably deduce that a class member or local variable is used before it's initialized. However, we should be aware that the analyzer can't guarantee this 100% of the time. The proposal only states that in this case you'll get Warnings are a bit more delicate matter as a balance is needed. We want to avoid false positives, because they annoy users and prevent them from finding useful warnings among the garbage. But false negative scenarios (no warning where it is needed) are also dangerous, they can completely negate the value of the warning due to its rarity, or give the user a false sense of security if they are not familiar with how static analysis works in a particular case. In my opinion, this is more related to the implementation phase and further improvements of static analysis. The only reason I'm pointing this out is because this proposal introduces invalid default values to the language.
One of my intentions with this proposal is to identify things that cannot be achieved in 4.x or that would be problematic to implement before 5.0. In my opinion, making objects non-nullable by default is one of those. This doesn't mean nullable types are unachievable in 4.x at all, I'm just skeptical that we can decouple If we look at the current type system from the point of view of set theory, then I imagine it something like this: So, I don't fully agree when someone says that If we wanted to introduce non-nullable object types in 4.x without compatibility breakage, we would need type intersection and complement operations. Formally, This would seem pretty consistent to me, although not perfect. You could use it for new code, but the engine API, plugins, and other existing code would be unaffected. However, typed arrays and dictionaries currently don't support nested types, if you want something like Footnotes
|
Thank you for clarifying your view, geekley. I wrote this assuming that a patch-by-patch fix for Godot 4 was compatible with a ground up redesign for 5.0, but if they're not, I agree with you that the latter is more important. While, I agree with most of your post, it's not clear to me why adding
This claim that class members can potentially be changed by any call is the exact premise that I’m disputing, dalexeev. When I asked, “how would an uninitialized object's data be accessed when no other part of the program knows knows [sic] about the uninitialized object's existence yet?”, the question was not purely rhetorical. Here's the basic argument describing my reasoning. I agree with you that the static analyzer will never be able know whether outside code would access an uninitialized object’s members if the outside code had a reference to that object, but I think the analyzer can know that outside code will access an uninitialized object’s members only if the code has a reference to the object. On my current understanding, the static analyzer can also be certain that no code outside of the initialization routine has a reference to the uninitialized object. This is possible because a script’s initialization routine (i.e. its initializers, Insofar as the analyzer really can be certain that 1) outside code will access data only if the code has a reference to the data and 2) no code outside of the initialization routine has a reference to an object, then it follows that in these situations, the analyzer can be also be certain that 3) outside code will never access that object. This is why I suspect that by ensuring that a script’s initialization routine never leaks any references to
I was thinking through these initialization questions in the context of a broader question about type safety and gradual typing. I thought my original post made sense without that context, but I see now that it didn’t. Currently, GDScript’s type system is weak enough that it is impossible to write a real game in GDScript without making unsafe calls all over the place (the lack of options for type narrowing is a good example of this). Even in places where type safe code is possible, doing so may require so much boilerplate that it’s currently impractical. Consequently, there’s really no point in making the compiler call out every individual instance of an unsafe function call; in any reasonably sized project, the compiler would mostly end up dredging up false positives. However, I suspect everyone in this thread would like to see GDScript’s type improved to the point where ordinary game logic can be written in completely type-safe GDScript. Should that glorious day ever arrive (and I'm talking about 5.0 features here), we fervent believers in static typing will want to be know wherever our code is unsafe and perhaps even want the compiler to ask for explicit permission to compile such code. If I forget an important type annotation, I want to know about it. At the same time, there’s a certain kind of person who doesn’t forget type annotations; they omit them. Everywhere. Those people, incorrigible though they are, still make up an important part of this community, and if the compiler were statically typed all the way through, the engine would become unusable for them. So the problem I was trying to think through was, “how can we meet the needs of those who want type annotations everywhere without alienating people who like permissive type systems?” The potential solution to this problem that I was exploring was to make the compiler as permissive as possible while raising extensive warnings that could be handled according to the particular user's needs. Basically, the compiler would fail only if it could prove that, given a set of "normal" assumptions (e.g. no calls to
Users would then be given the ability to configure the “strength” of the type checker by determining how type-check warnings should be handled (e.g. printed as-is, elevated to errors, or suppressed). The type checker’s “strength” could then be configured as a project wide configuration setting and then overridden at more specific levels. Some examples of cases where overriding the type checker's strength would be desirable are a project that is strongly typed except for a handful of files which were hacked together as proof of concept for a new feature, a project that is weakly typed except for a couple plugins, and a file that is strongly typed except for a couple lines of code that the author knows will never fail. I was interested in an approach like this was because it could be implemented without adding any configurability options to the compiler itself.
My concern here is that if we say that "declaring |
I can only speak for myself, but mainly because it would mean having 3 different possibilities: Even if you do have a setting to forbid plain It's just better if |
Note
This proposal breaks compatibility in a major way and is aimed at 5.0.
Describe the project you are working on
Proposals for improving Godot's type system and GDScript language. Providing null safety. This can be important for many projects.
Describe the problem or limitation you are having in your project
Valid and invalid empty/missing values
The
null
value (nil
,None
,undefined
, etc.) is used differently in different languages:NULL
andnullptr
are used to denote a null pointer. There is no primary (not a pointer)Null
type.null
is used as a special empty/missing value of a separate type. This avoids the use of special "empty" values within the same type, such as0
,-1
,""
, etc.In GDScript,
null
combines both of these qualities:null
to a function that expects an object, or assignnull
to a variable that is statically typed as an object.null
to a function that expects anint
.There are proposals for nullable (
int?
) and union (int|Nil
,int|String
) types, see #162 and #737. The fact that objects are nullable by default makes it difficult to implement a unified and consistent type system.Also, the current behavior has the problem that you can't distinguish when
null
denotes a valid empty/missing value from an invalid value (e.g. you forgot to initialize a variable). Given that dereferencing a null pointer in C++ is a fairly common error, the same error withnull
in GDScript is no less common. I often see user posts in various Godot groups asking aboutInvalid access to property or key 'foo' on a base object of type 'Nil'
andInvalid call. Nonexistent function 'foo' in base 'Nil'
errors.Let's say you have a function:
Here you would probably expect that the parameter
a
should always be a valid node, and the parameterb
can be either a valid node ornull
. However, you can calltest(null, null)
and it will not immediately cause an error. As long as you do not access the properties and methods ona
, you will not detect the error. You can assigna
to another variable and the error will be detected much later.If this is an important method, you can check the value manually, like this:
However, doing this in every method is tedious. As you can see, the two
null
s passed totest()
are different from each other. The firstnull
is an indicator of a bug in our code (parametera
should never benull
), while the secondnull
is a perfectly normal situation (null
is a valid value for parameterb
). It would be nice if the difference could be expressed through the type system, like this:Or like this (union types are a superset of nullable types):
Note that you could also not make the parameter
b
optional, but still specify that it can acceptnull
. You would just have to explicitly pass some value, either a node ornull
.Variant
representationHere,
Variant
refers to the Godot C++ class, not the GDScript top type.You might not know this, but there are actually three kinds of "null" values in Godot/GDScript:
null
,Object#null
, andFreed Object
. See also #10098. Here I insert a short description:All values in GDScript are
Variant
. It has two fields: type (TYPE_NIL
,TYPE_OBJECT
, etc.) and content (a union). Also Godot/GDScript does not have a garbage collector. This means that objects can be destroyed before all references to the object become unreachable. Therefore, we have several possibilities to have an "invalid object" (null
-like values):null
(type isTYPE_NIL
, content is zero)Object#null
(type isTYPE_OBJECT
, content is zero)Object#null
(Ref<T>()
) instead ofnull
Variant()
. For example,Array.get_typed_script()
.Freed Object
(type isTYPE_OBJECT
, content is non-zero, after the object is destroyed)ObjectID
inOBjectDB
. TheVariant
data does not change when the pointed object is removed from memory.This proposal only affects
null
(TYPE_NIL
), you can still passObject#null
andFreed Object
(TYPE_OBJECT
) to a function expecting an object, or assign them to a variable statically typed as an object. This is important, see below for details.Memory management
Why do
Object#null
andFreed Object
even exist? It has to do with memory management and lifetime of different types:int
,float
,Vector2
,Color
, etc.). They are copied entirely, as they fit in aVariant
.Variant
size (for example,String
). Under the hood, they use a reference counting mechanism and copy-on-write. However, this happens invisible to the user, so we can consider it an implementation detail.Array
,Dictionary
, and packed arrays). SeveralVariant
s can share a reference to the same array/dictionary, deletion occurs automatically when there are no more references to the array/dictionary. User-defined constructors/destructors are not allowed. The user has no way to directly influence the reference counter.RefCounted
has reference counting andNode
automatically deletes its children when deleted). Custom constructors and destructors are supported (in the form ofNOTIFICATION_PREDELETE
), forRefCounted
there is an option to influence the reference counter (methodsinit_ref()
,reference()
andunreference()
). At any time, an object can be deleted usingfree()
(or by zeroing the reference counter), and also refuse to be deleted (cancel_free()
). When an object is deleted, all remaining references to it become invalid, which is why aFreed Object
appears (there is no point in updating all variants referencing it for performance reasons).So objects, in general, have manual memory management. Automatic memory management for objects is usually implemented in languages with garbage collection (most scripting languages, C#), languages with lifetime and reference ownership analysis (Rust), or functional languages, where variables are immutable (but under the hood they usually also use garbage collection). Godot, as a game engine, avoids garbage collection, so I don't think we should expect fully automatic memory management.
If we make objects non-nullable, what about default initialization and memory management? What is
node
equal to here?Given the above, I think it would be a bad idea to automatically instantiate
node
here. We could probably require explicit initialization of aNode
variable (but notNode?
), sincenull
is not a valid value. However, there are still cases where we need some default value:I propose to use
Object#null
as a default value in this case. It may seem like it doesn't change anything, but there is a big difference in semantics.null
will be treated as a valid empty/missing value. There is a literalnull
and it will be considered normal to use, as long as it complies with the type system constraints. WhereasObject#null
andFreed Object
will be a sign of an error (you forgot to initialize the variable, accessed it before it was initialized, or after the object was deleted from memory). You should not and are unlikely to intentionally passObject#null
orFreed Object
to functions, unlikenull
. This way,null
will be spared its dual role and will only denote a valid empty/missing value.Describe the feature / enhancement and how it helps to overcome the problem or limitation
Here are the key points of this proposal:
null
where an object type is expected. This includes core, GDScript, C#, and GDExtension.Object
type (and its descendants) fromnull
toObject#null
. The default value of theT?
(orT|Nil
) type isnull
.null
should only denote a valid empty/missing value.Object#null
andFreed Object
should be treated as a sign of an error.Sprite2D.texture
property should be of typeTexture2D?
, notTexture2D
. If a method can return a node ornull
, its return type should beNode?
, notNode
. A built-in method should not returnObject#null
orFreed Object
, unless this is the result of incorrect user action.Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
Variant::can_convert()
andVariant::can_convert_strict()
.Variant
constructors and the binding system.GDScriptAnalyzer::check_type_compatibility()
andGDScriptDataType::is_type()
.If this enhancement will not be used often, can it be worked around with a few lines of script?
This affects the core and cannot be worked around with a few lines of script.
Is there a reason why this should be core and not an add-on in the asset library?
This affects the core and cannot be implemented as an add-on.
The text was updated successfully, but these errors were encountered: