From 0cb185927c1021f07c8c2111fc73b7f712ea393f Mon Sep 17 00:00:00 2001 From: George Marques Date: Thu, 26 Nov 2020 14:41:55 -0300 Subject: [PATCH] GDScript: Improve handling of operators - Use the new functions in Variant to determine the validity and resulting type of operators. - Split the operator function in codegen between binary and unary, since the unary ones have now a special requirement of having the second argument to be the NIL type when requesting info. --- modules/gdscript/gdscript_analyzer.cpp | 82 +++++----------------- modules/gdscript/gdscript_analyzer.h | 1 + modules/gdscript/gdscript_byte_codegen.cpp | 23 +++++- modules/gdscript/gdscript_byte_codegen.h | 3 +- modules/gdscript/gdscript_codegen.h | 3 +- modules/gdscript/gdscript_compiler.cpp | 26 +++---- 6 files changed, 56 insertions(+), 82 deletions(-) diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 851994eff38..4d9d01b261e 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -2720,7 +2720,7 @@ void GDScriptAnalyzer::reduce_unary_op(GDScriptParser::UnaryOpNode *p_unary_op) mark_node_unsafe(p_unary_op); } else { bool valid = false; - result = get_operation_type(p_unary_op->variant_op, p_unary_op->operand->get_datatype(), p_unary_op->operand->get_datatype(), valid, p_unary_op); + result = get_operation_type(p_unary_op->variant_op, p_unary_op->operand->get_datatype(), valid, p_unary_op); if (!valid) { push_error(vformat(R"(Invalid operand of type "%s" for unary operator "%s".)", p_unary_op->operand->get_datatype().to_string(), Variant::get_operator_name(p_unary_op->variant_op)), p_unary_op->operand); @@ -3086,81 +3086,31 @@ bool GDScriptAnalyzer::is_shadowing(GDScriptParser::IdentifierNode *p_local, con } #endif -GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, const GDScriptParser::DataType &p_b, bool &r_valid, const GDScriptParser::Node *p_source) { - // This function creates dummy variant values and apply the operation to those. Less error-prone than keeping a table of valid operations. +GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, bool &r_valid, const GDScriptParser::Node *p_source) { + // Unary version. + GDScriptParser::DataType nil_type; + nil_type.builtin_type = Variant::NIL; + return get_operation_type(p_operation, p_a, nil_type, r_valid, p_source); +} +GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, const GDScriptParser::DataType &p_b, bool &r_valid, const GDScriptParser::Node *p_source) { GDScriptParser::DataType result; result.kind = GDScriptParser::DataType::VARIANT; Variant::Type a_type = p_a.builtin_type; Variant::Type b_type = p_b.builtin_type; - Variant a; - REF a_ref; - if (a_type == Variant::OBJECT) { - a_ref.instance(); - a = a_ref; - } else { - Callable::CallError err; - Variant::construct(a_type, a, nullptr, 0, err); - if (err.error != Callable::CallError::CALL_OK) { - r_valid = false; - ERR_FAIL_V_MSG(result, vformat("Could not construct value of type %s", Variant::get_type_name(a_type))); - } - } - Variant b; - REF b_ref; - if (b_type == Variant::OBJECT) { - b_ref.instance(); - b = b_ref; - } else { - Callable::CallError err; - Variant::construct(b_type, b, nullptr, 0, err); - if (err.error != Callable::CallError::CALL_OK) { - r_valid = false; - ERR_FAIL_V_MSG(result, vformat("Could not construct value of type %s", Variant::get_type_name(b_type))); - } + Variant::ValidatedOperatorEvaluator op_eval = Variant::get_validated_operator_evaluator(p_operation, a_type, b_type); + + if (op_eval == nullptr) { + r_valid = false; + return result; } - // Avoid division by zero. - switch (b_type) { - case Variant::INT: - b = 1; - break; - case Variant::FLOAT: - b = 1.0; - break; - case Variant::VECTOR2: - b = Vector2(1.0, 1.0); - break; - case Variant::VECTOR2I: - b = Vector2i(1, 1); - break; - case Variant::VECTOR3: - b = Vector3(1.0, 1.0, 1.0); - break; - case Variant::VECTOR3I: - b = Vector3i(1, 1, 1); - break; - case Variant::COLOR: - b = Color(1.0, 1.0, 1.0, 1.0); - break; - default: - // No change needed. - break; - } + r_valid = true; - // Avoid error in formatting operator (%) where it doesn't find a placeholder. - if (a_type == Variant::STRING && b_type != Variant::ARRAY) { - a = String("%s"); - } - - Variant ret; - Variant::evaluate(p_operation, a, b, ret, r_valid); - - if (r_valid) { - return type_from_variant(ret, p_source); - } + result.kind = GDScriptParser::DataType::BUILTIN; + result.builtin_type = Variant::get_operator_return_type(p_operation, a_type, b_type); return result; } diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h index 0a952cc6217..7a06a61239f 100644 --- a/modules/gdscript/gdscript_analyzer.h +++ b/modules/gdscript/gdscript_analyzer.h @@ -102,6 +102,7 @@ class GDScriptAnalyzer { bool validate_call_arg(const List &p_par_types, int p_default_args_count, bool p_is_vararg, const GDScriptParser::CallNode *p_call); bool validate_call_arg(const MethodInfo &p_method, const GDScriptParser::CallNode *p_call); GDScriptParser::DataType get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, const GDScriptParser::DataType &p_b, bool &r_valid, const GDScriptParser::Node *p_source); + GDScriptParser::DataType get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, bool &r_valid, const GDScriptParser::Node *p_source); bool is_type_compatible(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion = false) const; void push_error(const String &p_message, const GDScriptParser::Node *p_origin); void mark_node_unsafe(const GDScriptParser::Node *p_node); diff --git a/modules/gdscript/gdscript_byte_codegen.cpp b/modules/gdscript/gdscript_byte_codegen.cpp index a81b3298ef9..7c20cda39c8 100644 --- a/modules/gdscript/gdscript_byte_codegen.cpp +++ b/modules/gdscript/gdscript_byte_codegen.cpp @@ -321,7 +321,28 @@ void GDScriptByteCodeGenerator::set_initial_line(int p_line) { #define IS_BUILTIN_TYPE(m_var, m_type) \ (m_var.type.has_type && m_var.type.kind == GDScriptDataType::BUILTIN && m_var.type.builtin_type == m_type) -void GDScriptByteCodeGenerator::write_operator(const Address &p_target, Variant::Operator p_operator, const Address &p_left_operand, const Address &p_right_operand) { +void GDScriptByteCodeGenerator::write_unary_operator(const Address &p_target, Variant::Operator p_operator, const Address &p_left_operand) { + if (HAS_BUILTIN_TYPE(p_left_operand)) { + // Gather specific operator. + Variant::ValidatedOperatorEvaluator op_func = Variant::get_validated_operator_evaluator(p_operator, p_left_operand.type.builtin_type, Variant::NIL); + + append(GDScriptFunction::OPCODE_OPERATOR_VALIDATED, 3); + append(p_left_operand); + append(Address()); + append(p_target); + append(op_func); + return; + } + + // No specific types, perform variant evaluation. + append(GDScriptFunction::OPCODE_OPERATOR, 3); + append(p_left_operand); + append(Address()); + append(p_target); + append(p_operator); +} + +void GDScriptByteCodeGenerator::write_binary_operator(const Address &p_target, Variant::Operator p_operator, const Address &p_left_operand, const Address &p_right_operand) { if (HAS_BUILTIN_TYPE(p_left_operand) && HAS_BUILTIN_TYPE(p_right_operand)) { // Gather specific operator. Variant::ValidatedOperatorEvaluator op_func = Variant::get_validated_operator_evaluator(p_operator, p_left_operand.type.builtin_type, p_right_operand.type.builtin_type); diff --git a/modules/gdscript/gdscript_byte_codegen.h b/modules/gdscript/gdscript_byte_codegen.h index 0173b7f820b..34d2bb60987 100644 --- a/modules/gdscript/gdscript_byte_codegen.h +++ b/modules/gdscript/gdscript_byte_codegen.h @@ -377,7 +377,8 @@ public: #endif virtual void set_initial_line(int p_line) override; - virtual void write_operator(const Address &p_target, Variant::Operator p_operator, const Address &p_left_operand, const Address &p_right_operand) override; + virtual void write_unary_operator(const Address &p_target, Variant::Operator p_operator, const Address &p_left_operand) override; + virtual void write_binary_operator(const Address &p_target, Variant::Operator p_operator, const Address &p_left_operand, const Address &p_right_operand) override; virtual void write_type_test(const Address &p_target, const Address &p_source, const Address &p_type) override; virtual void write_type_test_builtin(const Address &p_target, const Address &p_source, Variant::Type p_type) override; virtual void write_and_left_operand(const Address &p_left_operand) override; diff --git a/modules/gdscript/gdscript_codegen.h b/modules/gdscript/gdscript_codegen.h index decab3df0bf..35e326c61ff 100644 --- a/modules/gdscript/gdscript_codegen.h +++ b/modules/gdscript/gdscript_codegen.h @@ -98,7 +98,8 @@ public: // virtual void alloc_stack(int p_level) = 0; // Is this needed? // virtual void alloc_call(int p_arg_count) = 0; // This might be automatic from other functions. - virtual void write_operator(const Address &p_target, Variant::Operator p_operator, const Address &p_left_operand, const Address &p_right_operand) = 0; + virtual void write_unary_operator(const Address &p_target, Variant::Operator p_operator, const Address &p_left_operand) = 0; + virtual void write_binary_operator(const Address &p_target, Variant::Operator p_operator, const Address &p_left_operand, const Address &p_right_operand) = 0; virtual void write_type_test(const Address &p_target, const Address &p_source, const Address &p_type) = 0; virtual void write_type_test_builtin(const Address &p_target, const Address &p_source, Variant::Type p_type) = 0; virtual void write_and_left_operand(const Address &p_left_operand) = 0; diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index 3d327cebeb3..b41dc15324b 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -677,7 +677,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code return GDScriptCodeGenerator::Address(); } - gen->write_operator(result, unary->variant_op, operand, GDScriptCodeGenerator::Address()); + gen->write_unary_operator(result, unary->variant_op, operand); if (operand.mode == GDScriptCodeGenerator::Address::TEMPORARY) { gen->pop_temporary(); @@ -745,7 +745,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code GDScriptCodeGenerator::Address left_operand = _parse_expression(codegen, r_error, binary->left_operand); GDScriptCodeGenerator::Address right_operand = _parse_expression(codegen, r_error, binary->right_operand); - gen->write_operator(result, binary->variant_op, left_operand, right_operand); + gen->write_binary_operator(result, binary->variant_op, left_operand, right_operand); if (right_operand.mode == GDScriptCodeGenerator::Address::TEMPORARY) { gen->pop_temporary(); @@ -908,7 +908,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code } else { gen->write_get(value, key, prev_base); } - gen->write_operator(value, assignment->variant_op, value, assigned); + gen->write_binary_operator(value, assignment->variant_op, value, assigned); if (assigned.mode == GDScriptCodeGenerator::Address::TEMPORARY) { gen->pop_temporary(); } @@ -966,7 +966,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code if (assignment->operation != GDScriptParser::AssignmentNode::OP_NONE) { GDScriptCodeGenerator::Address member = codegen.add_temporary(); gen->write_get_member(member, name); - gen->write_operator(assigned, assignment->variant_op, member, assigned); + gen->write_binary_operator(assigned, assignment->variant_op, member, assigned); gen->pop_temporary(); } @@ -1016,7 +1016,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code if (assignment->operation != GDScriptParser::AssignmentNode::OP_NONE) { // Perform operation. op_result = codegen.add_temporary(); - gen->write_operator(op_result, assignment->variant_op, target, assigned); + gen->write_binary_operator(op_result, assignment->variant_op, target, assigned); } else { op_result = assigned; assigned = GDScriptCodeGenerator::Address(); @@ -1072,7 +1072,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c // Check type equality. GDScriptCodeGenerator::Address type_equality_addr = codegen.add_temporary(equality_type); - codegen.generator->write_operator(type_equality_addr, Variant::OP_EQUAL, p_type_addr, literal_type_addr); + codegen.generator->write_binary_operator(type_equality_addr, Variant::OP_EQUAL, p_type_addr, literal_type_addr); codegen.generator->write_and_left_operand(type_equality_addr); // Get literal. @@ -1083,7 +1083,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c // Check value equality. GDScriptCodeGenerator::Address equality_addr = codegen.add_temporary(equality_type); - codegen.generator->write_operator(equality_addr, Variant::OP_EQUAL, p_value_addr, literal_addr); + codegen.generator->write_binary_operator(equality_addr, Variant::OP_EQUAL, p_value_addr, literal_addr); codegen.generator->write_and_right_operand(equality_addr); // AND both together (reuse temporary location). @@ -1135,11 +1135,11 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c codegen.generator->write_call_builtin(result_addr, GDScriptFunctions::TYPE_OF, typeof_args); // Check type equality. - codegen.generator->write_operator(result_addr, Variant::OP_EQUAL, p_type_addr, result_addr); + codegen.generator->write_binary_operator(result_addr, Variant::OP_EQUAL, p_type_addr, result_addr); codegen.generator->write_and_left_operand(result_addr); // Check value equality. - codegen.generator->write_operator(result_addr, Variant::OP_EQUAL, p_value_addr, expr_addr); + codegen.generator->write_binary_operator(result_addr, Variant::OP_EQUAL, p_value_addr, expr_addr); codegen.generator->write_and_right_operand(equality_test_addr); // AND both type and value equality. @@ -1185,7 +1185,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c // Check type equality. GDScriptCodeGenerator::Address result_addr = codegen.add_temporary(temp_type); - codegen.generator->write_operator(result_addr, Variant::OP_EQUAL, p_type_addr, array_type_addr); + codegen.generator->write_binary_operator(result_addr, Variant::OP_EQUAL, p_type_addr, array_type_addr); codegen.generator->write_and_left_operand(result_addr); // Store pattern length in constant map. @@ -1201,7 +1201,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c // Test length compatibility. temp_type.builtin_type = Variant::BOOL; GDScriptCodeGenerator::Address length_compat_addr = codegen.add_temporary(temp_type); - codegen.generator->write_operator(length_compat_addr, p_pattern->rest_used ? Variant::OP_GREATER_EQUAL : Variant::OP_EQUAL, value_length_addr, array_length_addr); + codegen.generator->write_binary_operator(length_compat_addr, p_pattern->rest_used ? Variant::OP_GREATER_EQUAL : Variant::OP_EQUAL, value_length_addr, array_length_addr); codegen.generator->write_and_right_operand(length_compat_addr); // AND type and length check. @@ -1284,7 +1284,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c // Check type equality. GDScriptCodeGenerator::Address result_addr = codegen.add_temporary(temp_type); - codegen.generator->write_operator(result_addr, Variant::OP_EQUAL, p_type_addr, dict_type_addr); + codegen.generator->write_binary_operator(result_addr, Variant::OP_EQUAL, p_type_addr, dict_type_addr); codegen.generator->write_and_left_operand(result_addr); // Store pattern length in constant map. @@ -1300,7 +1300,7 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_match_pattern(CodeGen &c // Test length compatibility. temp_type.builtin_type = Variant::BOOL; GDScriptCodeGenerator::Address length_compat_addr = codegen.add_temporary(temp_type); - codegen.generator->write_operator(length_compat_addr, p_pattern->rest_used ? Variant::OP_GREATER_EQUAL : Variant::OP_EQUAL, value_length_addr, dict_length_addr); + codegen.generator->write_binary_operator(length_compat_addr, p_pattern->rest_used ? Variant::OP_GREATER_EQUAL : Variant::OP_EQUAL, value_length_addr, dict_length_addr); codegen.generator->write_and_right_operand(length_compat_addr); // AND type and length check.