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

Concatenating constant arrays always gives the same array reference #96152

Open
xorblo-doitus opened this issue Aug 27, 2024 · 3 comments
Open

Comments

@xorblo-doitus
Copy link

Tested versions

  • Reproducible in:
    • v4.3.stable.official [77dcf97]
    • v4.4.dev.gh [db76de5]
    • v4.1.3.stable.official [f06b6836a]
    • v4.0.alpha1.official [31a7ddb]
  • Not reproducible in: v3.5.3.stable.official [6c81413]

System information

Godot v4.3.stable - Windows 10.0.22631 - GLES3 (Compatibility) - NVIDIA GeForce RTX 4050 Laptop GPU (NVIDIA; 32.0.15.5612) - 13th Gen Intel(R) Core(TM) i7-13620H (16 Threads)

Issue description

When concatenating constant arrays, the result is always a reference to the same array, probably due to constant folding.

As a result of this, creating a new array by concatenating two constant arrays, modifying the seemingly newly created array, and using the same concatenation elsewhere (or later in another call of the same function) gives the modified concatenation.

Example:

const A = ["a"]
const B = ["b"]

(A + B).append("X")
print(
	"\nA:     ", A,
	"\nB:     ", B,
	"\nA + B: ", A + B
)

Result:

A:     ["a"]
B:     ["b"]
A + B: ["a", "b", "X"]

Expected:

A:     ["a"]
B:     ["b"]
A + B: ["a", "b"]

Workaround:
Concatenating a non-constant empty array truly creates a new array. (tested only in v4.3.stable.official [77dcf97])

(A + B + []).append("X")
print(
	"\nA:     ", A,
	"\nB:     ", B,
	"\nA + B: ", A + B + []
)

Steps to reproduce

The MRP contains:

  • An EditorScript (test_in_editor.gd) that executes the previous code sample on execution (ctrl+shift+X by default)
    • NB: The concatenation result seems to be reset at each execution, so each execution gives the same result.
  • A scene with a Button that runs the previous code sample. (It's the default one, so you can press F5 to start it)
    • NB: The concatenation result is not reset at each execution, so each button press adds another "X".

Minimal reproduction project (MRP)

const_array_concatenation.zip

@dalexeev dalexeev added the bug label Aug 27, 2024
@github-project-automation github-project-automation bot moved this to For team assessment in GDScript Issue Triage Aug 27, 2024
@dalexeev dalexeev moved this from For team assessment to Up for grabs in GDScript Issue Triage Aug 27, 2024
@Chubercik
Copy link
Contributor

Chubercik commented Aug 29, 2024

I can confirm this bug is still present as of v4.4.dev.custom_build [108c603].


Even if we hide the A + B expression behind a new const variable C, the issue persists:

@tool
extends EditorScript


const A = ["a"]
const B = ["b"]


func _run() -> void:
	const C = A + B
	C.append("X")
	print(
		"======================",
		"\nA:     ", A,
		"\nB:     ", B,
		"\nA + B: ", A + B
	)
======================
A:     ["a"]
B:     ["b"]
A + B: ["a", "b", "X"]

@Chubercik
Copy link
Contributor

Related PR that seems to have tackled the issue at hand, but didn't cover this specific problem:

@girdenis-p
Copy link
Contributor

girdenis-p commented Nov 21, 2024

This does seem to be caused by constant folding performed by the analyzer. I believe normally array addition is done at runtime because at compile time, due to folding, it would cause the arrays with the same content to have the same reference.

The reason why this expression is folded is because the identifiers are constant and so they go through this condition when reducing the addition:

if (p_binary_op->left_operand->is_constant && p_binary_op->right_operand->is_constant) {

We could add a check here to check that the builtin type of the identifier is safe to fold. I would be happy to do this but I think we need a bit more discussion because adding this would break compat since expressions like const C = A + B (where A and B are const arrays) would begin to error.

EDIT: I forgot to mention that we already don't treat addition of array literals as constant expressions e.g. const C = ["a"] + ["b"] currently errors, this is inconsistent with the current behaviour of const C = A + B not erroring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Up for grabs
Development

No branches or pull requests

5 participants