Skip to content

Commit

Permalink
GDScript: Fix some issues with assignments that involve untyped things
Browse files Browse the repository at this point in the history
  • Loading branch information
@vonagam
vonagam committed Dec 31, 2022
1 parent 8a98110 commit 5c868fd
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 79 deletions.
190 changes: 111 additions & 79 deletions modules/gdscript/gdscript_analyzer.cpp
View file
Expand Up @@ -2255,82 +2255,86 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
push_error("Cannot assign a new value to a constant.", p_assignment->assignee);
}

bool assignee_is_variant = assignee_type.is_variant();
bool assignee_is_hard = assignee_type.is_hard_type();
bool assigned_is_variant = assigned_value_type.is_variant();
bool assigned_is_hard = assigned_value_type.is_hard_type();
bool compatible = true;
bool downgrades_assignee = false;
bool downgrades_assigned = false;
GDScriptParser::DataType op_type = assigned_value_type;
if (p_assignment->operation != GDScriptParser::AssignmentNode::OP_NONE) {
op_type = get_operation_type(p_assignment->variant_op, assignee_type, assigned_value_type, compatible, p_assignment->assigned_value);

if (assignee_is_variant) {
// operation with variant assignee
mark_node_unsafe(p_assignment);
} else if (!compatible) {
// incompatible operation with hard types and non-variant assignee
mark_node_unsafe(p_assignment);
if (assigned_is_variant) {
// incompatible operation with hard non-variant assignee and hard variant assigned
p_assignment->use_conversion_assign = true;
} else {
// incompatible operation with hard non-variant types
push_error(vformat(R"(Invalid operands "%s" and "%s" for assignment operator.)", assignee_type.to_string(), assigned_value_type.to_string()), p_assignment);
}
} else if (op_type.type_source == GDScriptParser::DataType::UNDETECTED && !assigned_is_variant) {
// incompatible operation with non-variant types (at least one weak)
downgrades_assignee = !assignee_is_hard;
downgrades_assigned = !assigned_is_hard;
}
}
p_assignment->set_datatype(op_type);

if (assignee_type.is_hard_type() && !assignee_type.is_variant() && op_type.is_hard_type()) {
if (compatible) {
compatible = is_type_compatible(assignee_type, op_type, true, p_assignment->assigned_value);
if (!compatible) {
// Try reverse test since it can be a masked subtype.
if (!is_type_compatible(op_type, assignee_type, true)) {
push_error(vformat(R"(Cannot assign a value of type "%s" to a target of type "%s".)", assigned_value_type.to_string(), assignee_type.to_string()), p_assignment->assigned_value);
} else {
// TODO: Add warning.
mark_node_unsafe(p_assignment);
p_assignment->use_conversion_assign = true;
}
}
} else {
push_error(vformat(R"(Invalid operands "%s" and "%s" for assignment operator.)", assignee_type.to_string(), assigned_value_type.to_string()), p_assignment);
if (assignee_is_variant) {
if (!assignee_is_hard) {
// assigning with weak variant assignee
mark_node_unsafe(p_assignment);
}
} else if (assignee_type.is_hard_type() && !assignee_type.is_variant()) {
mark_node_unsafe(p_assignment);
p_assignment->use_conversion_assign = true;
} else {
mark_node_unsafe(p_assignment);
if (assignee_type.is_hard_type() && !assignee_type.is_variant()) {
if (assignee_is_hard && !assigned_is_hard) {
// assigning with hard non-variant assignee and weak assigned
mark_node_unsafe(p_assignment);
p_assignment->use_conversion_assign = true;
}
}

if (p_assignment->assignee->type == GDScriptParser::Node::IDENTIFIER) {
// Change source type so it's not wrongly detected later.
GDScriptParser::IdentifierNode *identifier = static_cast<GDScriptParser::IdentifierNode *>(p_assignment->assignee);

switch (identifier->source) {
case GDScriptParser::IdentifierNode::MEMBER_VARIABLE: {
GDScriptParser::DataType id_type = identifier->variable_source->get_datatype();
if (!id_type.is_hard_type()) {
id_type.kind = GDScriptParser::DataType::VARIANT;
id_type.type_source = GDScriptParser::DataType::UNDETECTED;
identifier->variable_source->set_datatype(id_type);
}
} break;
case GDScriptParser::IdentifierNode::FUNCTION_PARAMETER: {
GDScriptParser::DataType id_type = identifier->parameter_source->get_datatype();
if (!id_type.is_hard_type()) {
id_type.kind = GDScriptParser::DataType::VARIANT;
id_type.type_source = GDScriptParser::DataType::UNDETECTED;
identifier->parameter_source->set_datatype(id_type);
}
} break;
case GDScriptParser::IdentifierNode::LOCAL_VARIABLE: {
GDScriptParser::DataType id_type = identifier->variable_source->get_datatype();
if (!id_type.is_hard_type()) {
id_type.kind = GDScriptParser::DataType::VARIANT;
id_type.type_source = GDScriptParser::DataType::UNDETECTED;
identifier->variable_source->set_datatype(id_type);
downgrades_assigned = downgrades_assigned || (!assigned_is_variant && !is_type_compatible(assignee_type, op_type, true, p_assignment->assigned_value));
} else if (compatible) {
if (op_type.is_variant()) {
// assigning with non-variant assignee and variant result
mark_node_unsafe(p_assignment);
if (assignee_is_hard) {
// assigning with hard non-variant assignee and variant result
p_assignment->use_conversion_assign = true;
} else {
// assigning with weak non-variant assignee and variant result
downgrades_assignee = true;
}
} break;
case GDScriptParser::IdentifierNode::LOCAL_ITERATOR: {
GDScriptParser::DataType id_type = identifier->bind_source->get_datatype();
if (!id_type.is_hard_type()) {
id_type.kind = GDScriptParser::DataType::VARIANT;
id_type.type_source = GDScriptParser::DataType::UNDETECTED;
identifier->variable_source->set_datatype(id_type);
} else if (!is_type_compatible(assignee_type, op_type, assignee_is_hard, p_assignment->assigned_value)) {
// assigning with non-variant assignee and incompatible result
mark_node_unsafe(p_assignment);
if (assignee_is_hard) {
if (is_type_compatible(op_type, assignee_type, true, p_assignment->assigned_value)) {
// assigning with hard non-variant assignee and maybe compatible result
p_assignment->use_conversion_assign = true;
} else {
// assigning with hard non-variant assignee and incompatible result
push_error(vformat(R"(Cannot assign a value of type "%s" to a target of type "%s".)", assigned_value_type.to_string(), assignee_type.to_string()), p_assignment->assigned_value);
}
} else {
// assigning with weak non-variant assignee and incompatible result
downgrades_assignee = true;
}
} break;
default:
// Nothing to do.
break;
}
}
}

if (downgrades_assignee) {
downgrade_node_type_source(p_assignment->assignee);
}
if (downgrades_assigned) {
downgrade_node_type_source(p_assignment->assigned_value);
}

#ifdef DEBUG_ENABLED
if (p_assignment->assigned_value->type == GDScriptParser::Node::CALL && assigned_value_type.is_hard_type() && assigned_value_type.kind == GDScriptParser::DataType::BUILTIN && assigned_value_type.builtin_type == Variant::NIL) {
parser->push_warning(p_assignment->assigned_value, GDScriptWarning::VOID_ASSIGNMENT, static_cast<GDScriptParser::CallNode *>(p_assignment->assigned_value)->function_name);
Expand Down Expand Up @@ -2788,6 +2792,7 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
} else {
reduce_expression(subscript->base);
base_type = subscript->base->get_datatype();
is_self = subscript->base->type == GDScriptParser::Node::SELF;
}
} else {
// Invalid call. Error already sent in parser.
Expand Down Expand Up @@ -4366,9 +4371,6 @@ GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator
}

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;

Expand All @@ -4388,28 +4390,18 @@ GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator
}

Variant::ValidatedOperatorEvaluator op_eval = Variant::get_validated_operator_evaluator(p_operation, a_type, b_type);

bool hard_operation = p_a.is_hard_type() && p_b.is_hard_type();
bool validated = op_eval != nullptr;

if (hard_operation && !validated) {
r_valid = false;
return result;
} else if (hard_operation && validated) {
GDScriptParser::DataType result;
if (validated) {
r_valid = true;
result.type_source = GDScriptParser::DataType::ANNOTATED_INFERRED;
result.type_source = hard_operation ? GDScriptParser::DataType::ANNOTATED_INFERRED : GDScriptParser::DataType::INFERRED;
result.kind = GDScriptParser::DataType::BUILTIN;
result.builtin_type = Variant::get_operator_return_type(p_operation, a_type, b_type);
} else if (!hard_operation && !validated) {
r_valid = true;
result.type_source = GDScriptParser::DataType::UNDETECTED;
} else {
r_valid = !hard_operation;
result.kind = GDScriptParser::DataType::VARIANT;
result.builtin_type = Variant::NIL;
} else if (!hard_operation && validated) {
r_valid = true;
result.type_source = GDScriptParser::DataType::INFERRED;
result.kind = GDScriptParser::DataType::BUILTIN;
result.builtin_type = Variant::get_operator_return_type(p_operation, a_type, b_type);
}

return result;
Expand Down Expand Up @@ -4585,6 +4577,46 @@ void GDScriptAnalyzer::mark_node_unsafe(const GDScriptParser::Node *p_node) {
#endif
}

void GDScriptAnalyzer::downgrade_node_type_source(GDScriptParser::Node *p_node) {
GDScriptParser::IdentifierNode *identifier = nullptr;
if (p_node->type == GDScriptParser::Node::IDENTIFIER) {
identifier = static_cast<GDScriptParser::IdentifierNode *>(p_node);
} else if (p_node->type == GDScriptParser::Node::SUBSCRIPT) {
GDScriptParser::SubscriptNode *subscript = static_cast<GDScriptParser::SubscriptNode *>(p_node);
if (subscript->is_attribute) {
identifier = subscript->attribute;
}
}
if (identifier == nullptr) {
return;
}

GDScriptParser::Node *source = nullptr;
switch (identifier->source) {
case GDScriptParser::IdentifierNode::MEMBER_VARIABLE: {
source = identifier->variable_source;
} break;
case GDScriptParser::IdentifierNode::FUNCTION_PARAMETER: {
source = identifier->parameter_source;
} break;
case GDScriptParser::IdentifierNode::LOCAL_VARIABLE: {
source = identifier->variable_source;
} break;
case GDScriptParser::IdentifierNode::LOCAL_ITERATOR: {
source = identifier->bind_source;
} break;
default:
break;
}
if (source == nullptr) {
return;
}

GDScriptParser::DataType datatype;
datatype.kind = GDScriptParser::DataType::VARIANT;
source->set_datatype(datatype);
}

void GDScriptAnalyzer::mark_lambda_use_self() {
for (GDScriptParser::LambdaNode *lambda : lambda_stack) {
lambda->use_self = true;
Expand Down
1 change: 1 addition & 0 deletions modules/gdscript/gdscript_analyzer.h
View file
Expand Up @@ -119,6 +119,7 @@ class GDScriptAnalyzer {
bool is_type_compatible(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion = false, const GDScriptParser::Node *p_source_node = nullptr);
void push_error(const String &p_message, const GDScriptParser::Node *p_origin = nullptr);
void mark_node_unsafe(const GDScriptParser::Node *p_node);
void downgrade_node_type_source(GDScriptParser::Node *p_node);
void mark_lambda_use_self();
bool class_exists(const StringName &p_class) const;
Ref<GDScriptParserRef> get_parser_for(const String &p_path);
Expand Down
3 changes: 3 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/incompatible_assignment.gd
View file
@@ -0,0 +1,3 @@
func test():
var foo: bool = true
foo += 'bar'
2 changes: 2 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/errors/incompatible_assignment.out
View file
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Invalid operands "bool" and "String" for assignment operator.
19 changes: 19 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/features/assignments_with_untyped.gd
View file
@@ -0,0 +1,19 @@
func test():
var one_0 = 0
one_0 = 1
var one_1 := one_0
print(one_1)

var two: Variant = 0
two += 2
print(two)

var three_0: Variant = 1
var three_1: int = 2
three_0 += three_1
print(three_0)

var four_0: int = 3
var four_1: Variant = 1
four_0 += four_1
print(four_0)
5 changes: 5 additions & 0 deletions modules/gdscript/tests/scripts/analyzer/features/assignments_with_untyped.out
View file
@@ -0,0 +1,5 @@
GDTEST_OK
1
2
3
4

0 comments on commit 5c868fd

Please sign in to comment.