Merge pull request #71845 from vonagam/fix-read-only

GDScript: Fix test for read-only state of constants

core/variant/array.cpp

@@ -775,15 +775,9 @@ Variant Array::get_typed_script() const {
 	return _p->typed.script;
 }
 
-void Array::set_read_only(bool p_enable) {
-	if (p_enable == bool(_p->read_only != nullptr)) {
-		return;
-	}
-	if (p_enable) {
+void Array::make_read_only() {
+	if (_p->read_only == nullptr) {
 		_p->read_only = memnew(Variant);
-	} else {
-		memdelete(_p->read_only);
-		_p->read_only = nullptr;
 	}
 }
 

core/variant/array.h

@@ -127,7 +127,7 @@ public:
 	StringName get_typed_class_name() const;
 	Variant get_typed_script() const;
 
-	void set_read_only(bool p_enable);
+	void make_read_only();
 	bool is_read_only() const;
 
 	Array(const Array &p_base, uint32_t p_type, const StringName &p_class_name, const Variant &p_script);

core/variant/dictionary.cpp

@@ -333,15 +333,9 @@ Dictionary Dictionary::duplicate(bool p_deep) const {
 	return recursive_duplicate(p_deep, 0);
 }
 
-void Dictionary::set_read_only(bool p_enable) {
-	if (p_enable == bool(_p->read_only != nullptr)) {
-		return;
-	}
-	if (p_enable) {
+void Dictionary::make_read_only() {
+	if (_p->read_only == nullptr) {
 		_p->read_only = memnew(Variant);
-	} else {
-		memdelete(_p->read_only);
-		_p->read_only = nullptr;
 	}
 }
 bool Dictionary::is_read_only() const {

core/variant/dictionary.h

@@ -86,7 +86,7 @@ public:
 	Dictionary duplicate(bool p_deep = false) const;
 	Dictionary recursive_duplicate(bool p_deep, int recursion_count) const;
 
-	void set_read_only(bool p_enable);
+	void make_read_only();
 	bool is_read_only() const;
 
 	const void *id() const;

core/variant/variant_call.cpp

@@ -2179,6 +2179,8 @@ static void _register_variant_builtin_methods() {
 	bind_method(Dictionary, values, sarray(), varray());
 	bind_method(Dictionary, duplicate, sarray("deep"), varray(false));
 	bind_method(Dictionary, get, sarray("key", "default"), varray(Variant()));
+	bind_method(Dictionary, make_read_only, sarray(), varray());
+	bind_method(Dictionary, is_read_only, sarray(), varray());
 
 	/* Array */
 
@@ -2226,7 +2228,7 @@ static void _register_variant_builtin_methods() {
 	bind_method(Array, get_typed_builtin, sarray(), varray());
 	bind_method(Array, get_typed_class_name, sarray(), varray());
 	bind_method(Array, get_typed_script, sarray(), varray());
-	bind_method(Array, set_read_only, sarray("enable"), varray());
+	bind_method(Array, make_read_only, sarray(), varray());
 	bind_method(Array, is_read_only, sarray(), varray());
 
 	/* Byte Array */

doc/classes/Array.xml

@@ -392,7 +392,7 @@
 		<method name="is_read_only" qualifiers="const">
 			<return type="bool" />
 			<description>
-				Returns [code]true[/code] if the array is read-only. See [method set_read_only]. Arrays are automatically read-only if declared with [code]const[/code] keyword.
+				Returns [code]true[/code] if the array is read-only. See [method make_read_only]. Arrays are automatically read-only if declared with [code]const[/code] keyword.
 			</description>
 		</method>
 		<method name="is_typed" qualifiers="const">
@@ -401,6 +401,12 @@
 				Returns [code]true[/code] if the array is typed. Typed arrays can only store elements of their associated type and provide type safety for the [code][][/code] operator. Methods of typed array still return [Variant].
 			</description>
 		</method>
+		<method name="make_read_only">
+			<return type="void" />
+			<description>
+				Makes the array read-only, i.e. disabled modifying of the array's elements. Does not apply to nested content, e.g. content of nested arrays.
+			</description>
+		</method>
 		<method name="map" qualifiers="const">
 			<return type="Array" />
 			<param index="0" name="method" type="Callable" />
@@ -524,13 +530,6 @@
 				Searches the array in reverse order. Optionally, a start search index can be passed. If negative, the start index is considered relative to the end of the array.
 			</description>
 		</method>
-		<method name="set_read_only">
-			<return type="void" />
-			<param index="0" name="enable" type="bool" />
-			<description>
-				Makes the [Array] read-only, i.e. disabled modifying of the array's elements. Does not apply to nested content, e.g. content of nested arrays.
-			</description>
-		</method>
 		<method name="set_typed">
 			<return type="void" />
 			<param index="0" name="type" type="int" />

doc/classes/Dictionary.xml

@@ -271,12 +271,24 @@
 				Returns [code]true[/code] if the dictionary is empty (its size is [code]0[/code]). See also [method size].
 			</description>
 		</method>
+		<method name="is_read_only" qualifiers="const">
+			<return type="bool" />
+			<description>
+				Returns [code]true[/code] if the dictionary is read-only. See [method make_read_only]. Dictionaries are automatically read-only if declared with [code]const[/code] keyword.
+			</description>
+		</method>
 		<method name="keys" qualifiers="const">
 			<return type="Array" />
 			<description>
 				Returns the list of keys in the dictionary.
 			</description>
 		</method>
+		<method name="make_read_only">
+			<return type="void" />
+			<description>
+				Makes the dictionary read-only, i.e. disabled modifying of the dictionary's contents. Does not apply to nested content, e.g. content of nested dicitonaries.
+			</description>
+		</method>
 		<method name="merge">
 			<return type="void" />
 			<param index="0" name="dictionary" type="Dictionary" />

modules/gdscript/gdscript_analyzer.cpp

@@ -879,7 +879,7 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
 
 				current_enum = prev_enum;
 
-				dictionary.set_read_only(true);
+				dictionary.make_read_only();
 				member.m_enum->set_datatype(enum_type);
 				member.m_enum->dictionary = dictionary;
 
@@ -3892,7 +3892,7 @@ void GDScriptAnalyzer::const_fold_array(GDScriptParser::ArrayNode *p_array, bool
 		array[i] = p_array->elements[i]->reduced_value;
 	}
 	if (p_is_const) {
-		array.set_read_only(true);
+		array.make_read_only();
 	}
 	p_array->is_constant = true;
 	p_array->reduced_value = array;
@@ -3919,7 +3919,7 @@ void GDScriptAnalyzer::const_fold_dictionary(GDScriptParser::DictionaryNode *p_d
 		dict[element.key->reduced_value] = element.value->reduced_value;
 	}
 	if (p_is_const) {
-		dict.set_read_only(true);
+		dict.make_read_only();
 	}
 	p_dictionary->is_constant = true;
 	p_dictionary->reduced_value = dict;

modules/gdscript/tests/scripts/runtime/errors/constant_array_push_back.gd

@@ -1,4 +0,0 @@
-const array: Array = [0]
-
-func test():
-	array.push_back(0)

modules/gdscript/tests/scripts/runtime/errors/constant_array_push_back.out

@@ -1,7 +0,0 @@
-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.

modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.gd

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

modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.out

@@ -1,7 +0,0 @@
-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.

modules/gdscript/tests/scripts/runtime/features/constants_are_read_only.gd

@@ -0,0 +1,10 @@
+const array: Array = [0]
+const dictionary := {1: 2}
+
+@warning_ignore("assert_always_true")
+func test():
+	assert(array.is_read_only() == true)
+	assert(str(array) == '[0]')
+	assert(dictionary.is_read_only() == true)
+	assert(str(dictionary) == '{ 1: 2 }')
+	print('ok')

modules/gdscript/tests/scripts/runtime/features/constants_are_read_only.out

@@ -0,0 +1,2 @@
+GDTEST_OK
+ok