Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GDScript: Fix some issues with assignments that involve untyped things #70733

Merged
merged 1 commit into from Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
195 changes: 114 additions & 81 deletions modules/gdscript/gdscript_analyzer.cpp
Expand Up @@ -1573,6 +1573,9 @@ void GDScriptAnalyzer::resolve_assignable(GDScriptParser::AssignableNode *p_assi
if (initializer_type.is_variant() || !initializer_type.is_hard_type()) {
mark_node_unsafe(p_assignable->initializer);
p_assignable->use_conversion_assign = true;
if (!initializer_type.is_variant() && !is_type_compatible(specified_type, initializer_type, true, p_assignable->initializer)) {
downgrade_node_type_source(p_assignable->initializer);
}
} else if (!is_type_compatible(specified_type, initializer_type, true, p_assignable->initializer)) {
if (!is_constant && is_type_compatible(initializer_type, specified_type, true, p_assignable->initializer)) {
mark_node_unsafe(p_assignable->initializer);
Expand Down Expand Up @@ -2097,84 +2100,86 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig

GDScriptParser::DataType assigned_value_type = p_assignment->assigned_value->get_datatype();

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.is_variant()) {
op_type = get_operation_type(p_assignment->variant_op, assignee_type, assigned_value_type, compatible, p_assignment->assigned_value);

if (assignee_is_variant) {
// variant assignee
mark_node_unsafe(p_assignment);
} else if (!compatible) {
// incompatible hard types and non-variant assignee
mark_node_unsafe(p_assignment);
if (assigned_is_variant) {
// incompatible hard non-variant assignee and hard variant assigned
p_assignment->use_conversion_assign = true;
} else {
// incompatible 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 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 is a variant, then you can assign anything
// When the assigned value has a known type, further checks are possible.
if (assignee_type.is_hard_type() && !assignee_type.is_variant() && op_type.is_hard_type() && !op_type.is_variant()) {
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"(Value of type "%s" cannot be assigned to a variable 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) {
// 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) {
// 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()) {
// non-variant assignee and variant result
mark_node_unsafe(p_assignment);
if (assignee_is_hard) {
// hard non-variant assignee and variant result
p_assignment->use_conversion_assign = true;
} else {
// 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->bind_source->set_datatype(id_type);
} else if (!is_type_compatible(assignee_type, op_type, assignee_is_hard, p_assignment->assigned_value)) {
// 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)) {
// hard non-variant assignee and maybe compatible result
p_assignment->use_conversion_assign = true;
} else {
// hard non-variant assignee and incompatible result
push_error(vformat(R"(Value of type "%s" cannot be assigned to a variable of type "%s".)", assigned_value_type.to_string(), assignee_type.to_string()), p_assignment->assigned_value);
}
} else {
// 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 (assignee_type.is_hard_type() && assignee_type.builtin_type == Variant::INT && assigned_value_type.builtin_type == Variant::FLOAT) {
parser->push_warning(p_assignment->assigned_value, GDScriptWarning::NARROWING_CONVERSION);
Expand Down Expand Up @@ -2638,6 +2643,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 @@ -4235,9 +4241,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 @@ -4257,28 +4260,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 @@ -4454,6 +4447,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
Expand Up @@ -120,6 +120,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
@@ -0,0 +1,3 @@
func test():
var foo: bool = true
foo += 'bar'
@@ -0,0 +1,2 @@
GDTEST_ANALYZER_ERROR
Invalid operands "bool" and "String" for assignment operator.
@@ -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)
@@ -0,0 +1,5 @@
GDTEST_OK
1
2
3
4