Browse Source

GDScript: Begin making constants deep, not shallow or flat

Dmitrii Maganov 2 năm trước cách đây
mục cha
commit
5e2ac1a31e

+ 0 - 10
core/variant/array.cpp

@@ -54,16 +54,6 @@ void Array::_ref(const Array &p_from) const {
 
 
 	ERR_FAIL_COND(!_fp); // should NOT happen.
 	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) {
 	if (_fp == _p) {
 		return; // whatever it is, nothing to do here move along
 		return; // whatever it is, nothing to do here move along
 	}
 	}

+ 0 - 10
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 {
 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)
 	//make a copy first (thread safe)
 	if (!p_from._p->refcount.ref()) {
 	if (!p_from._p->refcount.ref()) {
 		return; // couldn't copy
 		return; // couldn't copy

+ 20 - 14
modules/gdscript/gdscript_analyzer.cpp

@@ -1507,9 +1507,9 @@ void GDScriptAnalyzer::resolve_assignable(GDScriptParser::AssignableNode *p_assi
 
 
 		if (is_constant) {
 		if (is_constant) {
 			if (p_assignable->initializer->type == GDScriptParser::Node::ARRAY) {
 			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) {
 			} 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) {
 			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);
 				push_error(vformat(R"(Assigned value for %s "%s" isn't a constant expression.)", p_kind, p_assignable->identifier->name), p_assignable->initializer);
@@ -2063,6 +2063,10 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
 
 
 	GDScriptParser::DataType assignee_type = p_assignment->assignee->get_datatype();
 	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.
 	// 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) {
 	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));
 		update_array_literal_element_type(assignee_type, static_cast<GDScriptParser::ArrayNode *>(p_assignment->assigned_value));
@@ -2070,10 +2074,6 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
 
 
 	GDScriptParser::DataType assigned_value_type = p_assignment->assigned_value->get_datatype();
 	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;
 	bool compatible = true;
 	GDScriptParser::DataType op_type = assigned_value_type;
 	GDScriptParser::DataType op_type = assigned_value_type;
 	if (p_assignment->operation != GDScriptParser::AssignmentNode::OP_NONE) {
 	if (p_assignment->operation != GDScriptParser::AssignmentNode::OP_NONE) {
@@ -3411,9 +3411,9 @@ void GDScriptAnalyzer::reduce_subscript(GDScriptParser::SubscriptNode *p_subscri
 		reduce_expression(p_subscript->base);
 		reduce_expression(p_subscript->base);
 
 
 		if (p_subscript->base->type == GDScriptParser::Node::ARRAY) {
 		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) {
 		} 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);
 		}
 		}
 	}
 	}
 
 
@@ -3738,16 +3738,16 @@ void GDScriptAnalyzer::reduce_unary_op(GDScriptParser::UnaryOpNode *p_unary_op)
 	p_unary_op->set_datatype(result);
 	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;
 	bool all_is_constant = true;
 
 
 	for (int i = 0; i < p_array->elements.size(); i++) {
 	for (int i = 0; i < p_array->elements.size(); i++) {
 		GDScriptParser::ExpressionNode *element = p_array->elements[i];
 		GDScriptParser::ExpressionNode *element = p_array->elements[i];
 
 
 		if (element->type == GDScriptParser::Node::ARRAY) {
 		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) {
 		} 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;
 		all_is_constant = all_is_constant && element->is_constant;
@@ -3761,20 +3761,23 @@ void GDScriptAnalyzer::const_fold_array(GDScriptParser::ArrayNode *p_array) {
 	for (int i = 0; i < p_array->elements.size(); i++) {
 	for (int i = 0; i < p_array->elements.size(); i++) {
 		array[i] = p_array->elements[i]->reduced_value;
 		array[i] = p_array->elements[i]->reduced_value;
 	}
 	}
+	if (p_is_const) {
+		array.set_read_only(true);
+	}
 	p_array->is_constant = true;
 	p_array->is_constant = true;
 	p_array->reduced_value = array;
 	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;
 	bool all_is_constant = true;
 
 
 	for (int i = 0; i < p_dictionary->elements.size(); i++) {
 	for (int i = 0; i < p_dictionary->elements.size(); i++) {
 		const GDScriptParser::DictionaryNode::Pair &element = p_dictionary->elements[i];
 		const GDScriptParser::DictionaryNode::Pair &element = p_dictionary->elements[i];
 
 
 		if (element.value->type == GDScriptParser::Node::ARRAY) {
 		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) {
 		} 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;
 		all_is_constant = all_is_constant && element.key->is_constant && element.value->is_constant;
@@ -3788,6 +3791,9 @@ void GDScriptAnalyzer::const_fold_dictionary(GDScriptParser::DictionaryNode *p_d
 		const GDScriptParser::DictionaryNode::Pair &element = p_dictionary->elements[i];
 		const GDScriptParser::DictionaryNode::Pair &element = p_dictionary->elements[i];
 		dict[element.key->reduced_value] = element.value->reduced_value;
 		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->is_constant = true;
 	p_dictionary->reduced_value = dict;
 	p_dictionary->reduced_value = dict;
 }
 }

+ 2 - 2
modules/gdscript/gdscript_analyzer.h

@@ -102,8 +102,8 @@ class GDScriptAnalyzer {
 	void reduce_ternary_op(GDScriptParser::TernaryOpNode *p_ternary_op);
 	void reduce_ternary_op(GDScriptParser::TernaryOpNode *p_ternary_op);
 	void reduce_unary_op(GDScriptParser::UnaryOpNode *p_unary_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.
 	// Helpers.
 	GDScriptParser::DataType type_from_variant(const Variant &p_value, const GDScriptParser::Node *p_source);
 	GDScriptParser::DataType type_from_variant(const Variant &p_value, const GDScriptParser::Node *p_source);

+ 5 - 0
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

+ 2 - 0
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.

+ 5 - 0
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

+ 2 - 0
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.

+ 6 - 0
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

+ 6 - 0
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'

+ 4 - 0
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)

+ 7 - 0
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.

+ 4 - 0
modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.gd

@@ -0,0 +1,4 @@
+const dictionary := {}
+
+func test():
+  dictionary.erase(0)

+ 7 - 0
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.

+ 6 - 0
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

+ 6 - 0
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'