Skip to content
This repository was archived by the owner on Sep 3, 2022. It is now read-only.

Commit 89eb6d3

Browse files
akien-mgaakien-mga
authored
Merge pull request godotengine#57591 from vnen/gdscript-enum-fixes
2 parents 225a3b2 + ceafdf3 commit 89eb6d3

34 files changed

+352
-57
lines changed

‎doc/classes/ProjectSettings.xml

+2
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,8 @@
356356
<member name="debug/gdscript/warnings/incompatible_ternary" type="bool" setter="" getter="" default="true">
357357
If [code]true[/code], enables warnings when a ternary operator may emit values with incompatible types.
358358
</member>
359+
<member name="debug/gdscript/warnings/int_assigned_to_enum" type="bool" setter="" getter="" default="true">
360+
</member>
359361
<member name="debug/gdscript/warnings/integer_division" type="bool" setter="" getter="" default="true">
360362
If [code]true[/code], enables warnings when dividing an integer by another integer (the decimal part will be discarded).
361363
</member>

‎modules/gdscript/gdscript_analyzer.cpp

+88-45
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ static GDScriptParser::DataType make_native_enum_type(const StringName &p_native
108108
GDScriptParser::DataType type;
109109
type.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;
110110
type.kind = GDScriptParser::DataType::ENUM;
111-
type.builtin_type = Variant::OBJECT;
111+
type.builtin_type = Variant::INT;
112112
type.is_constant = true;
113113
type.is_meta_type = true;
114114

@@ -650,9 +650,9 @@ void GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode *p_clas
650650
datatype = specified_type;
651651

652652
if (member.variable->initializer != nullptr) {
653-
if (!is_type_compatible(datatype, member.variable->initializer->get_datatype(), true)) {
653+
if (!is_type_compatible(datatype, member.variable->initializer->get_datatype(), true, member.variable->initializer)) {
654654
// Try reverse test since it can be a masked subtype.
655-
if (!is_type_compatible(member.variable->initializer->get_datatype(), datatype, true)) {
655+
if (!is_type_compatible(member.variable->initializer->get_datatype(), datatype, true, member.variable->initializer)) {
656656
push_error(vformat(R"(Value of type "%s" cannot be assigned to a variable of type "%s".)", member.variable->initializer->get_datatype().to_string(), datatype.to_string()), member.variable->initializer);
657657
} else {
658658
// TODO: Add warning.
@@ -1400,9 +1400,9 @@ void GDScriptAnalyzer::resolve_variable(GDScriptParser::VariableNode *p_variable
14001400
type.is_meta_type = false;
14011401

14021402
if (p_variable->initializer != nullptr) {
1403-
if (!is_type_compatible(type, p_variable->initializer->get_datatype(), true)) {
1403+
if (!is_type_compatible(type, p_variable->initializer->get_datatype(), true, p_variable->initializer)) {
14041404
// Try reverse test since it can be a masked subtype.
1405-
if (!is_type_compatible(p_variable->initializer->get_datatype(), type, true)) {
1405+
if (!is_type_compatible(p_variable->initializer->get_datatype(), type, true, p_variable->initializer)) {
14061406
push_error(vformat(R"(Value of type "%s" cannot be assigned to a variable of type "%s".)", p_variable->initializer->get_datatype().to_string(), type.to_string()), p_variable->initializer);
14071407
} else {
14081408
// TODO: Add warning.
@@ -1877,11 +1877,11 @@ void GDScriptAnalyzer::reduce_assignment(GDScriptParser::AssignmentNode *p_assig
18771877

18781878
if (!assignee_type.is_variant() && assigned_value_type.is_hard_type()) {
18791879
if (compatible) {
1880-
compatible = is_type_compatible(assignee_type, op_type, true);
1880+
compatible = is_type_compatible(assignee_type, op_type, true, p_assignment->assigned_value);
18811881
if (!compatible) {
18821882
if (assignee_type.is_hard_type()) {
18831883
// Try reverse test since it can be a masked subtype.
1884-
if (!is_type_compatible(op_type, assignee_type, true)) {
1884+
if (!is_type_compatible(op_type, assignee_type, true, p_assignment->assigned_value)) {
18851885
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);
18861886
} else {
18871887
// TODO: Add warning.
@@ -2416,6 +2416,11 @@ void GDScriptAnalyzer::reduce_call(GDScriptParser::CallNode *p_call, bool p_is_a
24162416
}
24172417
validate_call_arg(par_types, default_arg_count, is_vararg, p_call);
24182418

2419+
if (base_type.kind == GDScriptParser::DataType::ENUM && base_type.is_meta_type) {
2420+
// Enum type is treated as a dictionary value for function calls.
2421+
base_type.is_meta_type = false;
2422+
}
2423+
24192424
if (is_self && parser->current_function != nullptr && parser->current_function->is_static && !is_static) {
24202425
push_error(vformat(R"*(Cannot call non-static function "%s()" from static function "%s()".)*", p_call->function_name, parser->current_function->identifier->name), p_call->callee);
24212426
} else if (!is_self && base_type.is_meta_type && !is_static) {
@@ -2474,17 +2479,24 @@ void GDScriptAnalyzer::reduce_cast(GDScriptParser::CastNode *p_cast) {
24742479
GDScriptParser::DataType cast_type = resolve_datatype(p_cast->cast_type);
24752480

24762481
if (!cast_type.is_set()) {
2482+
mark_node_unsafe(p_cast);
24772483
return;
24782484
}
24792485

2480-
cast_type.is_meta_type = false; // The casted value won't be a type name.
2486+
cast_type = type_from_metatype(cast_type); // The casted value won't be a type name.
24812487
p_cast->set_datatype(cast_type);
24822488

24832489
if (!cast_type.is_variant()) {
24842490
GDScriptParser::DataType op_type = p_cast->operand->get_datatype();
24852491
if (!op_type.is_variant()) {
24862492
bool valid = false;
2487-
if (op_type.kind == GDScriptParser::DataType::BUILTIN && cast_type.kind == GDScriptParser::DataType::BUILTIN) {
2493+
if (op_type.kind == GDScriptParser::DataType::ENUM && cast_type.kind == GDScriptParser::DataType::ENUM) {
2494+
// Enum types are compatible between each other, so it's a safe cast.
2495+
valid = true;
2496+
} else if (op_type.kind == GDScriptParser::DataType::BUILTIN && op_type.builtin_type == Variant::INT && cast_type.kind == GDScriptParser::DataType::ENUM) {
2497+
// Convertint int to enum is always valid.
2498+
valid = true;
2499+
} else if (op_type.kind == GDScriptParser::DataType::BUILTIN && cast_type.kind == GDScriptParser::DataType::BUILTIN) {
24882500
valid = Variant::can_convert(op_type.builtin_type, cast_type.builtin_type);
24892501
} else if (op_type.kind != GDScriptParser::DataType::BUILTIN && cast_type.kind != GDScriptParser::DataType::BUILTIN) {
24902502
valid = is_type_compatible(cast_type, op_type) || is_type_compatible(op_type, cast_type);
@@ -2586,6 +2598,34 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
25862598

25872599
const StringName &name = p_identifier->name;
25882600

2601+
if (base.kind == GDScriptParser::DataType::ENUM) {
2602+
if (base.is_meta_type) {
2603+
if (base.enum_values.has(name)) {
2604+
p_identifier->is_constant = true;
2605+
p_identifier->reduced_value = base.enum_values[name];
2606+
2607+
GDScriptParser::DataType result;
2608+
result.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;
2609+
result.kind = GDScriptParser::DataType::ENUM;
2610+
result.is_constant = true;
2611+
result.builtin_type = Variant::INT;
2612+
result.native_type = base.native_type;
2613+
result.enum_type = base.enum_type;
2614+
p_identifier->set_datatype(result);
2615+
return;
2616+
} else {
2617+
// Consider as a Dictionary, so it can be anything.
2618+
// This will be evaluated in the next if block.
2619+
base.kind = GDScriptParser::DataType::BUILTIN;
2620+
base.builtin_type = Variant::DICTIONARY;
2621+
base.is_meta_type = false;
2622+
}
2623+
} else {
2624+
push_error(R"(Cannot get property from enum value.)", p_identifier);
2625+
return;
2626+
}
2627+
}
2628+
25892629
if (base.kind == GDScriptParser::DataType::BUILTIN) {
25902630
if (base.is_meta_type) {
25912631
bool valid = true;
@@ -2632,32 +2672,6 @@ void GDScriptAnalyzer::reduce_identifier_from_base(GDScriptParser::IdentifierNod
26322672
return;
26332673
}
26342674

2635-
if (base.kind == GDScriptParser::DataType::ENUM) {
2636-
if (base.is_meta_type) {
2637-
if (base.enum_values.has(name)) {
2638-
p_identifier->is_constant = true;
2639-
p_identifier->reduced_value = base.enum_values[name];
2640-
2641-
GDScriptParser::DataType result;
2642-
result.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;
2643-
result.kind = GDScriptParser::DataType::ENUM_VALUE;
2644-
result.is_constant = true;
2645-
result.builtin_type = Variant::INT;
2646-
result.native_type = base.native_type;
2647-
result.enum_type = name;
2648-
p_identifier->set_datatype(result);
2649-
} else {
2650-
// Consider as a Dictionary
2651-
GDScriptParser::DataType dummy;
2652-
dummy.kind = GDScriptParser::DataType::VARIANT;
2653-
p_identifier->set_datatype(dummy);
2654-
}
2655-
} else {
2656-
push_error(R"(Cannot get property from enum value.)", p_identifier);
2657-
}
2658-
return;
2659-
}
2660-
26612675
GDScriptParser::ClassNode *base_class = base.class_type;
26622676

26632677
// TODO: Switch current class/function/suite here to avoid misrepresenting identifiers (in recursive reduce calls).
@@ -2793,7 +2807,7 @@ void GDScriptAnalyzer::reduce_identifier(GDScriptParser::IdentifierNode *p_ident
27932807
if (element.identifier->name == p_identifier->name) {
27942808
GDScriptParser::DataType type;
27952809
type.type_source = GDScriptParser::DataType::ANNOTATED_EXPLICIT;
2796-
type.kind = element.parent_enum->identifier ? GDScriptParser::DataType::ENUM_VALUE : GDScriptParser::DataType::BUILTIN;
2810+
type.kind = element.parent_enum->identifier ? GDScriptParser::DataType::ENUM : GDScriptParser::DataType::BUILTIN;
27972811
type.builtin_type = Variant::INT;
27982812
type.is_constant = true;
27992813
if (element.parent_enum->identifier) {
@@ -3493,6 +3507,9 @@ GDScriptParser::DataType GDScriptAnalyzer::type_from_metatype(const GDScriptPars
34933507
GDScriptParser::DataType result = p_meta_type;
34943508
result.is_meta_type = false;
34953509
result.is_constant = false;
3510+
if (p_meta_type.kind == GDScriptParser::DataType::ENUM) {
3511+
result.builtin_type = Variant::INT;
3512+
}
34963513
return result;
34973514
}
34983515

@@ -3549,6 +3566,18 @@ bool GDScriptAnalyzer::get_function_signature(GDScriptParser::CallNode *p_source
35493566
r_default_arg_count = 0;
35503567
StringName function_name = p_function;
35513568

3569+
if (p_base_type.kind == GDScriptParser::DataType::ENUM) {
3570+
if (p_base_type.is_meta_type) {
3571+
// Enum type can be treated as a dictionary value.
3572+
p_base_type.kind = GDScriptParser::DataType::BUILTIN;
3573+
p_base_type.builtin_type = Variant::DICTIONARY;
3574+
p_base_type.is_meta_type = false;
3575+
} else {
3576+
push_error("Cannot call function on enum value.", p_source);
3577+
return false;
3578+
}
3579+
}
3580+
35523581
if (p_base_type.kind == GDScriptParser::DataType::BUILTIN) {
35533582
// Construct a base type to get methods.
35543583
Callable::CallError err;
@@ -3799,6 +3828,22 @@ GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator
37993828

38003829
Variant::Type a_type = p_a.builtin_type;
38013830
Variant::Type b_type = p_b.builtin_type;
3831+
3832+
if (p_a.kind == GDScriptParser::DataType::ENUM) {
3833+
if (p_a.is_meta_type) {
3834+
a_type = Variant::DICTIONARY;
3835+
} else {
3836+
a_type = Variant::INT;
3837+
}
3838+
}
3839+
if (p_b.kind == GDScriptParser::DataType::ENUM) {
3840+
if (p_b.is_meta_type) {
3841+
b_type = Variant::DICTIONARY;
3842+
} else {
3843+
b_type = Variant::INT;
3844+
}
3845+
}
3846+
38023847
Variant::ValidatedOperatorEvaluator op_eval = Variant::get_validated_operator_evaluator(p_operation, a_type, b_type);
38033848

38043849
bool hard_operation = p_a.is_hard_type() && p_b.is_hard_type();
@@ -3828,7 +3873,7 @@ GDScriptParser::DataType GDScriptAnalyzer::get_operation_type(Variant::Operator
38283873
}
38293874

38303875
// TODO: Add safe/unsafe return variable (for variant cases)
3831-
bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion) const {
3876+
bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion, const GDScriptParser::Node *p_source_node) {
38323877
// These return "true" so it doesn't affect users negatively.
38333878
ERR_FAIL_COND_V_MSG(!p_target.is_set(), true, "Parser bug (please report): Trying to check compatibility of unset target type");
38343879
ERR_FAIL_COND_V_MSG(!p_source.is_set(), true, "Parser bug (please report): Trying to check compatibility of unset value type");
@@ -3848,7 +3893,7 @@ bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_targ
38483893
if (!valid && p_allow_implicit_conversion) {
38493894
valid = Variant::can_convert_strict(p_source.builtin_type, p_target.builtin_type);
38503895
}
3851-
if (!valid && p_target.builtin_type == Variant::INT && p_source.kind == GDScriptParser::DataType::ENUM_VALUE) {
3896+
if (!valid && p_target.builtin_type == Variant::INT && p_source.kind == GDScriptParser::DataType::ENUM && !p_source.is_meta_type) {
38523897
// Enum value is also integer.
38533898
valid = true;
38543899
}
@@ -3869,18 +3914,18 @@ bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_targ
38693914

38703915
if (p_target.kind == GDScriptParser::DataType::ENUM) {
38713916
if (p_source.kind == GDScriptParser::DataType::BUILTIN && p_source.builtin_type == Variant::INT) {
3917+
#ifdef DEBUG_ENABLED
3918+
if (p_source_node) {
3919+
parser->push_warning(p_source_node, GDScriptWarning::INT_ASSIGNED_TO_ENUM);
3920+
}
3921+
#endif
38723922
return true;
38733923
}
38743924
if (p_source.kind == GDScriptParser::DataType::ENUM) {
38753925
if (p_source.native_type == p_target.native_type) {
38763926
return true;
38773927
}
38783928
}
3879-
if (p_source.kind == GDScriptParser::DataType::ENUM_VALUE) {
3880-
if (p_source.native_type == p_target.native_type && p_target.enum_values.has(p_source.enum_type)) {
3881-
return true;
3882-
}
3883-
}
38843929
return false;
38853930
}
38863931

@@ -3935,7 +3980,6 @@ bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_targ
39353980
case GDScriptParser::DataType::VARIANT:
39363981
case GDScriptParser::DataType::BUILTIN:
39373982
case GDScriptParser::DataType::ENUM:
3938-
case GDScriptParser::DataType::ENUM_VALUE:
39393983
case GDScriptParser::DataType::UNRESOLVED:
39403984
break; // Already solved before.
39413985
}
@@ -3972,7 +4016,6 @@ bool GDScriptAnalyzer::is_type_compatible(const GDScriptParser::DataType &p_targ
39724016
case GDScriptParser::DataType::VARIANT:
39734017
case GDScriptParser::DataType::BUILTIN:
39744018
case GDScriptParser::DataType::ENUM:
3975-
case GDScriptParser::DataType::ENUM_VALUE:
39764019
case GDScriptParser::DataType::UNRESOLVED:
39774020
break; // Already solved before.
39784021
}

‎modules/gdscript/gdscript_analyzer.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ class GDScriptAnalyzer {
112112
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);
113113
GDScriptParser::DataType get_operation_type(Variant::Operator p_operation, const GDScriptParser::DataType &p_a, bool &r_valid, const GDScriptParser::Node *p_source);
114114
void update_array_literal_element_type(const GDScriptParser::DataType &p_base_type, GDScriptParser::ArrayNode *p_array_literal);
115-
bool is_type_compatible(const GDScriptParser::DataType &p_target, const GDScriptParser::DataType &p_source, bool p_allow_implicit_conversion = false) const;
115+
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);
116116
void push_error(const String &p_message, const GDScriptParser::Node *p_origin);
117117
void mark_node_unsafe(const GDScriptParser::Node *p_node);
118118
bool class_exists(const StringName &p_class) const;

‎modules/gdscript/gdscript_compiler.cpp

+13-3
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,13 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
141141
}
142142
} break;
143143
case GDScriptParser::DataType::ENUM:
144-
case GDScriptParser::DataType::ENUM_VALUE:
145144
result.has_type = true;
146145
result.kind = GDScriptDataType::BUILTIN;
147-
result.builtin_type = Variant::INT;
146+
if (p_datatype.is_meta_type) {
147+
result.builtin_type = Variant::DICTIONARY;
148+
} else {
149+
result.builtin_type = Variant::INT;
150+
}
148151
break;
149152
case GDScriptParser::DataType::UNRESOLVED: {
150153
ERR_PRINT("Parser bug: converting unresolved type.");
@@ -469,7 +472,14 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code
469472
} break;
470473
case GDScriptParser::Node::CAST: {
471474
const GDScriptParser::CastNode *cn = static_cast<const GDScriptParser::CastNode *>(p_expression);
472-
GDScriptDataType cast_type = _gdtype_from_datatype(cn->cast_type->get_datatype());
475+
GDScriptParser::DataType og_cast_type = cn->cast_type->get_datatype();
476+
GDScriptDataType cast_type = _gdtype_from_datatype(og_cast_type);
477+
478+
if (og_cast_type.kind == GDScriptParser::DataType::ENUM) {
479+
// Enum types are usually treated as dictionaries, but in this case we want to cast to an integer.
480+
cast_type.kind = GDScriptDataType::BUILTIN;
481+
cast_type.builtin_type = Variant::INT;
482+
}
473483

474484
// Create temporary for result first since it will be deleted last.
475485
GDScriptCodeGenerator::Address result = codegen.add_temporary(cast_type);

‎modules/gdscript/gdscript_editor.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ static String _make_arguments_hint(const GDScriptParser::FunctionNode *p_functio
610610
case GDScriptParser::Node::SUBSCRIPT: {
611611
const GDScriptParser::SubscriptNode *sub = static_cast<const GDScriptParser::SubscriptNode *>(par->default_value);
612612
if (sub->is_constant) {
613-
if (sub->datatype.kind == GDScriptParser::DataType::ENUM_VALUE) {
613+
if (sub->datatype.kind == GDScriptParser::DataType::ENUM) {
614614
def_val = sub->get_datatype().to_string();
615615
} else if (sub->reduced) {
616616
const Variant::Type vt = sub->reduced_value.get_type();

‎modules/gdscript/gdscript_parser.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -3740,8 +3740,6 @@ String GDScriptParser::DataType::to_string() const {
37403740
}
37413741
case ENUM:
37423742
return enum_type.operator String() + " (enum)";
3743-
case ENUM_VALUE:
3744-
return enum_type.operator String() + " (enum value)";
37453743
case UNRESOLVED:
37463744
return "<unresolved type>";
37473745
}

‎modules/gdscript/gdscript_parser.h

+1-4
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,7 @@ class GDScriptParser {
106106
NATIVE,
107107
SCRIPT,
108108
CLASS, // GDScript.
109-
ENUM, // Full enumeration.
110-
ENUM_VALUE, // Value from enumeration.
109+
ENUM, // Enumeration.
111110
VARIANT, // Can be any type.
112111
UNRESOLVED,
113112
};
@@ -185,8 +184,6 @@ class GDScriptParser {
185184
return builtin_type == p_other.builtin_type;
186185
case NATIVE:
187186
case ENUM:
188-
return native_type == p_other.native_type;
189-
case ENUM_VALUE:
190187
return native_type == p_other.native_type && enum_type == p_other.enum_type;
191188
case SCRIPT:
192189
return script_type == p_other.script_type;

‎modules/gdscript/gdscript_warning.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ String GDScriptWarning::get_message() const {
152152
CHECK_SYMBOLS(3);
153153
return vformat(R"(The %s '%s' has the same name as a %s.)", symbols[0], symbols[1], symbols[2]);
154154
}
155+
case INT_ASSIGNED_TO_ENUM: {
156+
return "Integer used when an enum value is expected. If this is intended cast the integer to the enum type.";
157+
}
155158
case WARNING_MAX:
156159
break; // Can't happen, but silences warning
157160
}
@@ -199,6 +202,7 @@ String GDScriptWarning::get_name_from_code(Code p_code) {
199202
"REDUNDANT_AWAIT",
200203
"EMPTY_FILE",
201204
"SHADOWED_GLOBAL_IDENTIFIER",
205+
"INT_ASSIGNED_TO_ENUM",
202206
};
203207

204208
static_assert((sizeof(names) / sizeof(*names)) == WARNING_MAX, "Amount of warning types don't match the amount of warning names.");

‎modules/gdscript/gdscript_warning.h

+1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ class GDScriptWarning {
7070
REDUNDANT_AWAIT, // await is used but expression is synchronous (not a signal nor a coroutine).
7171
EMPTY_FILE, // A script file is empty.
7272
SHADOWED_GLOBAL_IDENTIFIER, // A global class or function has the same name as variable.
73+
INT_ASSIGNED_TO_ENUM, // An integer value was assigned to an enum-typed variable without casting.
7374
WARNING_MAX,
7475
};
7576

0 commit comments

Comments
 (0)