Merge pull request #71051 from vonagam/consts-are-deep-start

GDScript: Begin making constants deep, not shallow or flat

16 files changed, 82 insertions, 36 deletions

diff --git a/core/variant/array.cpp b/core/variant/array.cpp
index 0fecc2fe94..f8af78f3c1 100644
--- a/core/variant/array.cpp
+++ b/core/variant/array.cpp
@@ -54,16 +54,6 @@ void Array::_ref(const Array &p_from) const {
 
 	ERR_FAIL_COND(!_fp); // should NOT happen.
 
-	if (unlikely(_fp->read_only != nullptr)) {
-		// If p_from is a read-only array, just copy the contents to avoid further modification.
-		_unref();
-		_p = memnew(ArrayPrivate);
-		_p->refcount.init();
-		_p->array = _fp->array;
-		_p->typed = _fp->typed;
-		return;
-	}
-
 	if (_fp == _p) {
 		return; // whatever it is, nothing to do here move along
 	}
diff --git a/core/variant/dictionary.cpp b/core/variant/dictionary.cpp
index c545109bd8..f87064a0d1 100644
--- a/core/variant/dictionary.cpp
+++ b/core/variant/dictionary.cpp
@@ -211,16 +211,6 @@ bool Dictionary::recursive_equal(const Dictionary &p_dictionary, int recursion_c
 }
 
 void Dictionary::_ref(const Dictionary &p_from) const {
-	if (unlikely(p_from._p->read_only != nullptr)) {
-		// If p_from is a read-only dictionary, just copy the contents to avoid further modification.
-		if (_p) {
-			_unref();
-		}
-		_p = memnew(DictionaryPrivate);
-		_p->refcount.init();
-		_p->variant_map = p_from._p->variant_map;
-		return;
-	}
 	//make a copy first (thread safe)
 	if (!p_from._p->refcount.ref()) {
 		return; // couldn't copy
diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp
index 3455c69ad5..c2fd3b8a5d 100644
--- a/modules/gdscript/gdscript_analyzer.cpp
+++ b/modules/gdscript/gdscript_analyzer.cpp
@@ -1537,9 +1537,9 @@ void GDScriptAnalyzer::resolve_assignable(GDScriptParser::AssignableNode *p_assi
 
 		if (is_constant) {
 			if (p_assignable->initializer->type == GDScriptParser::Node::ARRAY) {
-				const_fold_array(static_cast<GDScriptParser::ArrayNode *>(p_assignable->initializer));
+				const_fold_array(static_cast<GDScriptParser::ArrayNode *>(p_assignable->initializer), true);
 			} else if (p_assignable->initializer->type == GDScriptParser::Node::DICTIONARY) {
-				const_fold_dictionary(static_cast<GDScriptParser::DictionaryNode *>(p_assignable->initializer));
+				const_fold_dictionary(static_cast<GDScriptParser::DictionaryNode *>(p_assignable->initializer), true);
 			}
 			if (!p_assignable->initializer->is_constant) {
 				push_error(vformat(R"(Assigned value for %s "%s" isn't a constant expression.)", p_kind, p_assignable->identifier->name), p_assignable->initializer);
@@ -2093,6 +2093,10 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
 
 	GDScriptParser::DataType assignee_type = p_assignment->assignee->get_datatype();
 
+	if (assignee_type.is_constant || (p_assignment->assignee->type == GDScriptParser::Node::SUBSCRIPT && static_cast<GDScriptParser::SubscriptNode *>(p_assignment->assignee)->base->is_constant)) {
+		push_error("Cannot assign a new value to a constant.", p_assignment->assignee);
+	}
+
 	// Check if assigned value is an array literal, so we can make it a typed array too if appropriate.
 	if (assignee_type.has_container_element_type() && p_assignment->assigned_value->type == GDScriptParser::Node::ARRAY) {
 		update_array_literal_element_type(assignee_type, static_cast<GDScriptParser::ArrayNode *>(p_assignment->assigned_value));
@@ -2100,10 +2104,6 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
 
 	GDScriptParser::DataType assigned_value_type = p_assignment->assigned_value->get_datatype();
 
-	if (assignee_type.is_constant) {
-		push_error("Cannot assign a new value to a constant.", p_assignment->assignee);
-	}
-
 	bool compatible = true;
 	GDScriptParser::DataType op_type = assigned_value_type;
 	if (p_assignment->operation != GDScriptParser::AssignmentNode::OP_NONE) {
@@ -3472,9 +3472,9 @@ void GDScriptAnalyzer::reduce_subscript(GDScriptParser::SubscriptNode *p_subscri
 		reduce_expression(p_subscript->base);
 
 		if (p_subscript->base->type == GDScriptParser::Node::ARRAY) {
-			const_fold_array(static_cast<GDScriptParser::ArrayNode *>(p_subscript->base));
+			const_fold_array(static_cast<GDScriptParser::ArrayNode *>(p_subscript->base), false);
 		} else if (p_subscript->base->type == GDScriptParser::Node::DICTIONARY) {
-			const_fold_dictionary(static_cast<GDScriptParser::DictionaryNode *>(p_subscript->base));
+			const_fold_dictionary(static_cast<GDScriptParser::DictionaryNode *>(p_subscript->base), false);
 		}
 	}
 
@@ -3799,16 +3799,16 @@ void GDScriptAnalyzer::reduce_unary_op(GDScriptParser::UnaryOpNode *p_unary_op)
 	p_unary_op->set_datatype(result);
 }
 
-void GDScriptAnalyzer::const_fold_array(GDScriptParser::ArrayNode *p_array) {
+void GDScriptAnalyzer::const_fold_array(GDScriptParser::ArrayNode *p_array, bool p_is_const) {
 	bool all_is_constant = true;
 
 	for (int i = 0; i < p_array->elements.size(); i++) {
 		GDScriptParser::ExpressionNode *element = p_array->elements[i];
 
 		if (element->type == GDScriptParser::Node::ARRAY) {
-			const_fold_array(static_cast<GDScriptParser::ArrayNode *>(element));
+			const_fold_array(static_cast<GDScriptParser::ArrayNode *>(element), p_is_const);
 		} else if (element->type == GDScriptParser::Node::DICTIONARY) {
-			const_fold_dictionary(static_cast<GDScriptParser::DictionaryNode *>(element));
+			const_fold_dictionary(static_cast<GDScriptParser::DictionaryNode *>(element), p_is_const);
 		}
 
 		all_is_constant = all_is_constant && element->is_constant;
@@ -3822,20 +3822,23 @@ void GDScriptAnalyzer::const_fold_array(GDScriptParser::ArrayNode *p_array) {
 	for (int i = 0; i < p_array->elements.size(); i++) {
 		array[i] = p_array->elements[i]->reduced_value;
 	}
+	if (p_is_const) {
+		array.set_read_only(true);
+	}
 	p_array->is_constant = true;
 	p_array->reduced_value = array;
 }
 
-void GDScriptAnalyzer::const_fold_dictionary(GDScriptParser::DictionaryNode *p_dictionary) {
+void GDScriptAnalyzer::const_fold_dictionary(GDScriptParser::DictionaryNode *p_dictionary, bool p_is_const) {
 	bool all_is_constant = true;
 
 	for (int i = 0; i < p_dictionary->elements.size(); i++) {
 		const GDScriptParser::DictionaryNode::Pair &element = p_dictionary->elements[i];
 
 		if (element.value->type == GDScriptParser::Node::ARRAY) {
-			const_fold_array(static_cast<GDScriptParser::ArrayNode *>(element.value));
+			const_fold_array(static_cast<GDScriptParser::ArrayNode *>(element.value), p_is_const);
 		} else if (element.value->type == GDScriptParser::Node::DICTIONARY) {
-			const_fold_dictionary(static_cast<GDScriptParser::DictionaryNode *>(element.value));
+			const_fold_dictionary(static_cast<GDScriptParser::DictionaryNode *>(element.value), p_is_const);
 		}
 
 		all_is_constant = all_is_constant && element.key->is_constant && element.value->is_constant;
@@ -3849,6 +3852,9 @@ void GDScriptAnalyzer::const_fold_dictionary(GDScriptParser::DictionaryNode *p_d
 		const GDScriptParser::DictionaryNode::Pair &element = p_dictionary->elements[i];
 		dict[element.key->reduced_value] = element.value->reduced_value;
 	}
+	if (p_is_const) {
+		dict.set_read_only(true);
+	}
 	p_dictionary->is_constant = true;
 	p_dictionary->reduced_value = dict;
 }
diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h
index 211ae3e7c2..a90a70dd9b 100644
--- a/modules/gdscript/gdscript_analyzer.h
+++ b/modules/gdscript/gdscript_analyzer.h
@@ -102,8 +102,8 @@ class GDScriptAnalyzer {
 	void reduce_ternary_op(GDScriptParser::TernaryOpNode *p_ternary_op);
 	void reduce_unary_op(GDScriptParser::UnaryOpNode *p_unary_op);
 
-	void const_fold_array(GDScriptParser::ArrayNode *p_array);
-	void const_fold_dictionary(GDScriptParser::DictionaryNode *p_dictionary);
+	void const_fold_array(GDScriptParser::ArrayNode *p_array, bool p_is_const);
+	void const_fold_dictionary(GDScriptParser::DictionaryNode *p_dictionary, bool p_is_const);
 
 	// Helpers.
 	GDScriptParser::DataType type_from_variant(const Variant &p_value, const GDScriptParser::Node *p_source);
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/constant_array_index_assign.gd b/modules/gdscript/tests/scripts/analyzer/errors/constant_array_index_assign.gd
new file mode 100644
index 0000000000..b8603dd4ca
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/constant_array_index_assign.gd
@@ -0,0 +1,5 @@
+const array: Array = [0]
+
+func test():
+	var key: int = 0
+	array[key] = 0
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/constant_array_index_assign.out b/modules/gdscript/tests/scripts/analyzer/errors/constant_array_index_assign.out
new file mode 100644
index 0000000000..5275183da2
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/constant_array_index_assign.out
@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Cannot assign a new value to a constant.
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/constant_dictionary_index_assign.gd b/modules/gdscript/tests/scripts/analyzer/errors/constant_dictionary_index_assign.gd
new file mode 100644
index 0000000000..9b5112b788
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/constant_dictionary_index_assign.gd
@@ -0,0 +1,5 @@
+const dictionary := {}
+
+func test():
+	var key: int = 0
+	dictionary[key] = 0
diff --git a/modules/gdscript/tests/scripts/analyzer/errors/constant_dictionary_index_assign.out b/modules/gdscript/tests/scripts/analyzer/errors/constant_dictionary_index_assign.out
new file mode 100644
index 0000000000..5275183da2
--- /dev/null
+++ b/modules/gdscript/tests/scripts/analyzer/errors/constant_dictionary_index_assign.out
@@ -0,0 +1,2 @@
+GDTEST_ANALYZER_ERROR
+Cannot assign a new value to a constant.
diff --git a/modules/gdscript/tests/scripts/runtime/errors/constant_array_is_deep.gd b/modules/gdscript/tests/scripts/runtime/errors/constant_array_is_deep.gd
new file mode 100644
index 0000000000..a5ecaba38d
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/errors/constant_array_is_deep.gd
@@ -0,0 +1,6 @@
+const array: Array = [{}]
+
+func test():
+	var dictionary := array[0]
+	var key: int = 0
+	dictionary[key] = 0
diff --git a/modules/gdscript/tests/scripts/runtime/errors/constant_array_is_deep.out b/modules/gdscript/tests/scripts/runtime/errors/constant_array_is_deep.out
new file mode 100644
index 0000000000..2a97eaea44
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/errors/constant_array_is_deep.out
@@ -0,0 +1,6 @@
+GDTEST_RUNTIME_ERROR
+>> SCRIPT ERROR
+>> on function: test()
+>> runtime/errors/constant_array_is_deep.gd
+>> 6
+>> Invalid set index '0' (on base: 'Dictionary') with value of type 'int'
diff --git a/modules/gdscript/tests/scripts/runtime/errors/constant_array_push_back.gd b/modules/gdscript/tests/scripts/runtime/errors/constant_array_push_back.gd
new file mode 100644
index 0000000000..3e71cd0518
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/errors/constant_array_push_back.gd
@@ -0,0 +1,4 @@
+const array: Array = [0]
+
+func test():
+	array.push_back(0)
diff --git a/modules/gdscript/tests/scripts/runtime/errors/constant_array_push_back.out b/modules/gdscript/tests/scripts/runtime/errors/constant_array_push_back.out
new file mode 100644
index 0000000000..ba3e1c46c6
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/errors/constant_array_push_back.out
@@ -0,0 +1,7 @@
+GDTEST_RUNTIME_ERROR
+>> ERROR
+>> on function: push_back()
+>> core/variant/array.cpp
+>> 253
+>> Condition "_p->read_only" is true.
+>> Array is in read-only state.
diff --git a/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.gd b/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.gd
new file mode 100644
index 0000000000..935fb773dc
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.gd
@@ -0,0 +1,4 @@
+const dictionary := {}
+
+func test():
+  dictionary.erase(0)
diff --git a/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.out b/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.out
new file mode 100644
index 0000000000..3e7ca11a4f
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.out
@@ -0,0 +1,7 @@
+GDTEST_RUNTIME_ERROR
+>> ERROR
+>> on function: erase()
+>> core/variant/dictionary.cpp
+>> 177
+>> Condition "_p->read_only" is true. Returning: false
+>> Dictionary is in read-only state.
diff --git a/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_is_deep.gd b/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_is_deep.gd
new file mode 100644
index 0000000000..4763210a7f
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_is_deep.gd
@@ -0,0 +1,6 @@
+const dictionary := {0: [0]}
+
+func test():
+	var array := dictionary[0]
+	var key: int = 0
+	array[key] = 0
diff --git a/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_is_deep.out b/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_is_deep.out
new file mode 100644
index 0000000000..c807db6b0c
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_is_deep.out
@@ -0,0 +1,6 @@
+GDTEST_RUNTIME_ERROR
+>> SCRIPT ERROR
+>> on function: test()
+>> runtime/errors/constant_dictionary_is_deep.gd
+>> 6
+>> Invalid set index '0' (on base: 'Array') with value of type 'int'