From 5e2ac1a31ee34842438a3a76c54f6a15df77bb95 Mon Sep 17 00:00:00 2001 From: Dmitrii Maganov Date: Sun, 8 Jan 2023 05:41:06 +0200 Subject: [PATCH] GDScript: Begin making constants deep, not shallow or flat --- core/variant/array.cpp | 10 ------ core/variant/dictionary.cpp | 10 ------ modules/gdscript/gdscript_analyzer.cpp | 34 +++++++++++-------- modules/gdscript/gdscript_analyzer.h | 4 +-- .../errors/constant_array_index_assign.gd | 5 +++ .../errors/constant_array_index_assign.out | 2 ++ .../constant_dictionary_index_assign.gd | 5 +++ .../constant_dictionary_index_assign.out | 2 ++ .../runtime/errors/constant_array_is_deep.gd | 6 ++++ .../runtime/errors/constant_array_is_deep.out | 6 ++++ .../errors/constant_array_push_back.gd | 4 +++ .../errors/constant_array_push_back.out | 7 ++++ .../errors/constant_dictionary_erase.gd | 4 +++ .../errors/constant_dictionary_erase.out | 7 ++++ .../errors/constant_dictionary_is_deep.gd | 6 ++++ .../errors/constant_dictionary_is_deep.out | 6 ++++ 16 files changed, 82 insertions(+), 36 deletions(-) create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/constant_array_index_assign.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/constant_array_index_assign.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/constant_dictionary_index_assign.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/constant_dictionary_index_assign.out create mode 100644 modules/gdscript/tests/scripts/runtime/errors/constant_array_is_deep.gd create mode 100644 modules/gdscript/tests/scripts/runtime/errors/constant_array_is_deep.out create mode 100644 modules/gdscript/tests/scripts/runtime/errors/constant_array_push_back.gd create mode 100644 modules/gdscript/tests/scripts/runtime/errors/constant_array_push_back.out create mode 100644 modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.gd create mode 100644 modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_erase.out create mode 100644 modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_is_deep.gd create mode 100644 modules/gdscript/tests/scripts/runtime/errors/constant_dictionary_is_deep.out diff --git a/core/variant/array.cpp b/core/variant/array.cpp index 0fecc2fe946b..f8af78f3c1fb 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 c545109bd832..f87064a0d1d9 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 8a07d509a102..e55b16c55d29 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -1507,9 +1507,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(p_assignable->initializer)); + const_fold_array(static_cast(p_assignable->initializer), true); } else if (p_assignable->initializer->type == GDScriptParser::Node::DICTIONARY) { - const_fold_dictionary(static_cast(p_assignable->initializer)); + const_fold_dictionary(static_cast(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); @@ -2063,6 +2063,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(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(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(); - 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) { @@ -3411,9 +3411,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(p_subscript->base)); + const_fold_array(static_cast(p_subscript->base), false); } else if (p_subscript->base->type == GDScriptParser::Node::DICTIONARY) { - const_fold_dictionary(static_cast(p_subscript->base)); + const_fold_dictionary(static_cast(p_subscript->base), false); } } @@ -3738,16 +3738,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(element)); + const_fold_array(static_cast(element), p_is_const); } else if (element->type == GDScriptParser::Node::DICTIONARY) { - const_fold_dictionary(static_cast(element)); + const_fold_dictionary(static_cast(element), p_is_const); } 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++) { 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(element.value)); + const_fold_array(static_cast(element.value), p_is_const); } else if (element.value->type == GDScriptParser::Node::DICTIONARY) { - const_fold_dictionary(static_cast(element.value)); + const_fold_dictionary(static_cast(element.value), p_is_const); } 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]; 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 a7f8e3b556fb..020c5f9022ec 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 000000000000..b8603dd4ca5f --- /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 000000000000..5275183da277 --- /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 000000000000..9b5112b7881c --- /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 000000000000..5275183da277 --- /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 000000000000..a5ecaba38df7 --- /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 000000000000..2a97eaea4423 --- /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 000000000000..3e71cd05187a --- /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 000000000000..ba3e1c46c609 --- /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 000000000000..935fb773dc13 --- /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 000000000000..3e7ca11a4fe3 --- /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 000000000000..4763210a7f1a --- /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 000000000000..c807db6b0c7e --- /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'