GDScript 2.0: type assignment is broken with Godot4 beta9

[MikeSchulze]

Godot version

v4.0.beta9.official [e780dc3]

System information

Windows 10

Issue description

After upgrade from Godot4 beta 8 to beta 9, my scripts are broken by script parsing errors.
It looks like the type assignment are broken.

Steps to reproduce

Godot Engine v4.0.beta9.official (c) 2007-2022 Juan Linietsky, Ariel Manzur & Godot Contributors.
modules/gltf/register_types.cpp:61 - Blend file import is enabled in the project settings, but no Blender path is configured in the editor settings. Blend files will not be imported.
--- Debug adapter server started ---
--- GDScript language server started ---
res://test.gd:6 - Parse Error: Could not infer the type of the variable "arg_default" because the initial value is a variant. Use explicit "Variant" type if this is intended.
res://test.gd:11 - Parse Error: Could not infer the type of the variable "x2" because the initial value is a variant. Use explicit "Variant" type if this is intended.

Minimal reproduction project

assign_bug.zip

[anvilfolk]

The two errors appear to be two different issues caused by two different commits.

The first error var arg_default := UNDEFINED appears to be due to #69471 cc @rune-scape who can probably fix it way faster than I could :)

Will bisect the other issue and come back :)

[anvilfolk]

Second issue is from #69518 cc @rune-scape

Sorry to keep throwing stuff at you :) At least I hope some of these are helpful to track stuff down. I don't know that I have it in me for right now.

[rune-scape]

hmm, maybe my change in #69518 wasn't right.
in GDScriptParser::DataType a built-in type of Variant::NIL means void or null, depending on the context, and when a function return a null literal it got reinterpreted as void, not a null Variant
i changed it thinking that it was reserved for the void type and a constant variant would be the same, but that loses type info that reduce_binary_op() needs to know the return type, the change should have been in decide_suite_type() instead

[vonagam]

I have #69163 which should fix the second issue.

[anvilfolk]

Good stuff, sounds like we're well on our way to getting these issues fixed :)

[rune-scape]

as for the first one, i didn't change the code responsible for that error, it should have given the same error in the last beta based on this:

godot/modules/gdscript/gdscript_analyzer.cpp

Lines 825 to 826 in 2a04b18

	} else if (datatype.is_variant()) {
	push_error(vformat(R"(Cannot infer the type of "%s" variable because the initial value is Variant. Use explicit "Variant" type if this is intended.)", member.variable->identifier->name), member.variable->initializer);

and this:

godot/modules/gdscript/gdscript_analyzer.cpp

Lines 1755 to 1756 in 2a04b18

	} else if (type.is_variant()) {
	push_error(vformat(R"(Could not infer the type of the variable "%s" because the initial value is a variant. Use explicit "Variant" type if this is intended.)", p_variable->identifier->name), p_variable->initializer);

it seems like collapsing this is_variant() and the has_no_type() branch into an is_hard_type() check would make more sense, and fix the issue

[vonagam]

I have not touched the first issue in that PR because the error explicitly said that inference should not work with Variant (it does not say anything about explicitly or implicitly specified). So I left it as it is. Cool if it will be allowed for explicit variants.

[rune-scape]

I have #69163 which should fix the second issue.

@vonagam i tested the build artifact with the MRP and it doesn't fix the issue, you'd need to check if they are reduced constants, then check the type of the reduced value
but i think reverting the null as variant part of #69518
instead of adding a null check to reduce_binary_op() would be better

[anvilfolk]

as for the first one, i didn't change the code responsible for that error, it should have given the same error in the last beta based on this:

I did test that it worked on beta 8 and didn't on beta 9, and did a git bisect which did identify the out-of-order member resolution PR as the introduction of regression for the first thing. I was really confused too, because being able to resolve things in different orders shouldn't change their types :/

[vonagam]

My mistake, yes, other things are broken because null is not built-in Nil now:

var x := null

It works when it should not, because other parts of code expect null to be kind == GDScriptParser::DataType::BUILTIN && builtin_type == Variant::NIL.

Since token is not Variant my PR is not required for it to work (it would be if token was Variant).

Edit: I updated my PR to allow var x := hard_variant.

[anvilfolk]

@vonagam might be worth tagging this as partially closed in your PR as well! I just reviewed that and it looks good, plus partially deals with this situation! 👍

[MikeSchulze]

👍

[MikeSchulze]

This bug is still dear in beta 10!
v4.0.beta10.official [d0398f6]

extends Node


const UNDEFINED :Variant = "<-NO_ARG->"

func _init(name :String, type :int = TYPE_MAX, default_value :Variant = UNDEFINED):
	pass

static func _extract_args() -> void:
	var arg_default := UNDEFINED

res://new_script.gd:10 - Parse Error: Could not infer the type of the variable "arg_default" because the initial value is a variant. Use explicit "Variant" type if this is intended.

[vonagam]

Well, there were two errors, they were fixed in two different PRs.
One of them got merged and that close the issue.
Another one - #69163 - is still waiting for that.

[MikeSchulze]

gotcha 👍