Skip to content

Commit 79ae952

Browse files
committed
Merge pull request #95115 from rune-scape/fix-invalidated-parser
GDScript: Fix unnecessary calls to `remove_parser`
2 parents ee986b7 + e680369 commit 79ae952

File tree

3 files changed

+35
-27
lines changed

3 files changed

+35
-27
lines changed

modules/gdscript/gdscript_analyzer.cpp

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -299,12 +299,14 @@ void GDScriptAnalyzer::get_class_node_current_scope_classes(GDScriptParser::Clas
299299

300300
// Prioritize node base type over its outer class
301301
if (p_node->base_type.class_type != nullptr) {
302-
ensure_cached_parser_for_class(p_node->base_type.class_type, p_node, vformat(R"(Trying to fetch classes in the current scope: base class of "%s")", p_node->fqcn), p_source);
302+
// TODO: 'ensure_cached_external_parser_for_class()' is only necessary because 'resolve_class_inheritance()' is not getting called here.
303+
ensure_cached_external_parser_for_class(p_node->base_type.class_type, p_node, "Trying to fetch classes in the current scope", p_source);
303304
get_class_node_current_scope_classes(p_node->base_type.class_type, p_list, p_source);
304305
}
305306

306307
if (p_node->outer != nullptr) {
307-
ensure_cached_parser_for_class(p_node->outer, p_node, vformat(R"(Trying to fetch classes in the current scope: outer class of "%s")", p_node->fqcn), p_source);
308+
// TODO: 'ensure_cached_external_parser_for_class()' is only necessary because 'resolve_class_inheritance()' is not getting called here.
309+
ensure_cached_external_parser_for_class(p_node->outer, p_node, "Trying to fetch classes in the current scope", p_source);
308310
get_class_node_current_scope_classes(p_node->outer, p_list, p_source);
309311
}
310312
}
@@ -314,10 +316,10 @@ Error GDScriptAnalyzer::resolve_class_inheritance(GDScriptParser::ClassNode *p_c
314316
p_source = p_class;
315317
}
316318

317-
Ref<GDScriptParserRef> parser_ref = ensure_cached_parser_for_class(p_class, nullptr, vformat(R"(Trying to resolve class inheritance of "%s")", p_class->fqcn), p_source);
319+
Ref<GDScriptParserRef> parser_ref = ensure_cached_external_parser_for_class(p_class, nullptr, "Trying to resolve class inheritance", p_source);
318320
Finally finally([&]() {
319321
for (GDScriptParser::ClassNode *look_class = p_class; look_class != nullptr; look_class = look_class->base_type.class_type) {
320-
ensure_cached_parser_for_class(look_class->base_type.class_type, look_class, vformat(R"(Trying to resolve class inheritance of "%s")", p_class->fqcn), p_source);
322+
ensure_cached_external_parser_for_class(look_class->base_type.class_type, look_class, "Trying to resolve class inheritance", p_source);
321323
}
322324
});
323325

@@ -888,12 +890,12 @@ void GDScriptAnalyzer::resolve_class_member(GDScriptParser::ClassNode *p_class,
888890
p_source = member.get_source_node();
889891
}
890892

891-
Ref<GDScriptParserRef> parser_ref = ensure_cached_parser_for_class(p_class, nullptr, vformat(R"(Trying to resolve class member "%s" of "%s")", member.get_name(), p_class->fqcn), p_source);
893+
Ref<GDScriptParserRef> parser_ref = ensure_cached_external_parser_for_class(p_class, nullptr, "Trying to resolve class member", p_source);
892894
Finally finally([&]() {
893-
ensure_cached_parser_for_class(member.get_datatype().class_type, p_class, vformat(R"(Trying to resolve datatype of class member "%s" of "%s")", member.get_name(), p_class->fqcn), p_source);
895+
ensure_cached_external_parser_for_class(member.get_datatype().class_type, p_class, "Trying to resolve datatype of class member", p_source);
894896
GDScriptParser::DataType member_type = member.get_datatype();
895897
if (member_type.has_container_element_type(0)) {
896-
ensure_cached_parser_for_class(member_type.get_container_element_type(0).class_type, p_class, vformat(R"(Trying to resolve datatype of class member "%s" of "%s")", member.get_name(), p_class->fqcn), p_source);
898+
ensure_cached_external_parser_for_class(member_type.get_container_element_type(0).class_type, p_class, "Trying to resolve datatype of class member", p_source);
897899
}
898900
});
899901

@@ -1181,7 +1183,7 @@ void GDScriptAnalyzer::resolve_class_interface(GDScriptParser::ClassNode *p_clas
11811183
p_source = p_class;
11821184
}
11831185

1184-
Ref<GDScriptParserRef> parser_ref = ensure_cached_parser_for_class(p_class, nullptr, vformat(R"(Trying to resolve class interface of "%s")", p_class->fqcn), p_source);
1186+
Ref<GDScriptParserRef> parser_ref = ensure_cached_external_parser_for_class(p_class, nullptr, "Trying to resolve class interface", p_source);
11851187

11861188
if (!p_class->resolved_interface) {
11871189
#ifdef DEBUG_ENABLED
@@ -1271,7 +1273,7 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co
12711273
p_source = p_class;
12721274
}
12731275

1274-
Ref<GDScriptParserRef> parser_ref = ensure_cached_parser_for_class(p_class, nullptr, vformat(R"(Trying to resolve class body of "%s")", p_class->fqcn), p_source);
1276+
Ref<GDScriptParserRef> parser_ref = ensure_cached_external_parser_for_class(p_class, nullptr, "Trying to resolve class body", p_source);
12751277

12761278
if (p_class->resolved_body) {
12771279
return;
@@ -3654,19 +3656,19 @@ GDScriptParser::DataType GDScriptAnalyzer::make_global_class_meta_type(const Str
36543656
}
36553657
}
36563658

3657-
Ref<GDScriptParserRef> GDScriptAnalyzer::ensure_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, const GDScriptParser::ClassNode *p_from_class, const String &p_context, const GDScriptParser::Node *p_source) {
3659+
Ref<GDScriptParserRef> GDScriptAnalyzer::ensure_cached_external_parser_for_class(const GDScriptParser::ClassNode *p_class, const GDScriptParser::ClassNode *p_from_class, const char *p_context, const GDScriptParser::Node *p_source) {
36583660
if (p_class == nullptr) {
36593661
return nullptr;
36603662
}
36613663

3662-
if (parser->has_class(p_class)) {
3663-
return nullptr;
3664-
}
3665-
36663664
if (HashMap<const GDScriptParser::ClassNode *, Ref<GDScriptParserRef>>::Iterator E = external_class_parser_cache.find(p_class)) {
36673665
return E->value;
36683666
}
36693667

3668+
if (parser->has_class(p_class)) {
3669+
return nullptr;
3670+
}
3671+
36703672
if (p_from_class == nullptr) {
36713673
p_from_class = parser->head;
36723674
}
@@ -3676,39 +3678,39 @@ Ref<GDScriptParserRef> GDScriptAnalyzer::ensure_cached_parser_for_class(const GD
36763678
Ref<GDScriptParserRef> parser_ref;
36773679
for (const GDScriptParser::ClassNode *look_class = p_from_class; look_class != nullptr; look_class = look_class->base_type.class_type) {
36783680
if (parser->has_class(look_class)) {
3679-
parser_ref = find_cached_parser_for_class(p_class, parser);
3681+
parser_ref = find_cached_external_parser_for_class(p_class, parser);
36803682
if (parser_ref.is_valid()) {
36813683
break;
36823684
}
36833685
}
36843686

36853687
if (HashMap<const GDScriptParser::ClassNode *, Ref<GDScriptParserRef>>::Iterator E = external_class_parser_cache.find(look_class)) {
3686-
parser_ref = find_cached_parser_for_class(p_class, E->value);
3688+
parser_ref = find_cached_external_parser_for_class(p_class, E->value);
36873689
if (parser_ref.is_valid()) {
36883690
break;
36893691
}
36903692
}
36913693

36923694
String look_class_script_path = look_class->get_datatype().script_path;
36933695
if (HashMap<String, Ref<GDScriptParserRef>>::Iterator E = parser->depended_parsers.find(look_class_script_path)) {
3694-
parser_ref = find_cached_parser_for_class(p_class, E->value);
3696+
parser_ref = find_cached_external_parser_for_class(p_class, E->value);
36953697
if (parser_ref.is_valid()) {
36963698
break;
36973699
}
36983700
}
36993701
}
37003702

37013703
if (parser_ref.is_null()) {
3702-
push_error(vformat(R"(Parser bug: Could not find external parser. (%s))", p_context), p_source);
3703-
// A null parser will be inserted into the cache and this error won't spam for the same class.
3704+
push_error(vformat(R"(Parser bug (please report): Could not find external parser for class "%s". (%s))", p_class->fqcn, p_context), p_source);
3705+
// A null parser will be inserted into the cache, so this error won't spam for the same class.
37043706
// This is ok, the values of external_class_parser_cache are not assumed to be valid references.
37053707
}
37063708

37073709
external_class_parser_cache.insert(p_class, parser_ref);
37083710
return parser_ref;
37093711
}
37103712

3711-
Ref<GDScriptParserRef> GDScriptAnalyzer::find_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, const Ref<GDScriptParserRef> &p_dependant_parser) {
3713+
Ref<GDScriptParserRef> GDScriptAnalyzer::find_cached_external_parser_for_class(const GDScriptParser::ClassNode *p_class, const Ref<GDScriptParserRef> &p_dependant_parser) {
37123714
if (p_dependant_parser.is_null()) {
37133715
return nullptr;
37143716
}
@@ -3729,10 +3731,10 @@ Ref<GDScriptParserRef> GDScriptAnalyzer::find_cached_parser_for_class(const GDSc
37293731

37303732
// Silently ensure it's parsed.
37313733
p_dependant_parser->raise_status(GDScriptParserRef::PARSED);
3732-
return find_cached_parser_for_class(p_class, p_dependant_parser->get_parser());
3734+
return find_cached_external_parser_for_class(p_class, p_dependant_parser->get_parser());
37333735
}
37343736

3735-
Ref<GDScriptParserRef> GDScriptAnalyzer::find_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, GDScriptParser *p_dependant_parser) {
3737+
Ref<GDScriptParserRef> GDScriptAnalyzer::find_cached_external_parser_for_class(const GDScriptParser::ClassNode *p_class, GDScriptParser *p_dependant_parser) {
37363738
if (p_dependant_parser == nullptr) {
37373739
return nullptr;
37383740
}
@@ -4474,7 +4476,11 @@ void GDScriptAnalyzer::reduce_preload(GDScriptParser::PreloadNode *p_preload) {
44744476
p_preload->is_constant = true;
44754477
p_preload->reduced_value = p_preload->resource;
44764478
p_preload->set_datatype(type_from_variant(p_preload->reduced_value, p_preload));
4477-
ensure_cached_parser_for_class(p_preload->get_datatype().class_type, nullptr, vformat(R"(Trying to resolve preload of "%s")", p_preload->resolved_path), p_preload);
4479+
4480+
// TODO: Not sure if this is necessary anymore.
4481+
// 'type_from_variant()' should call 'resolve_class_inheritance()' which would call 'ensure_cached_external_parser_for_class()'
4482+
// Better safe than sorry.
4483+
ensure_cached_external_parser_for_class(p_preload->get_datatype().class_type, nullptr, "Trying to resolve preload", p_preload);
44784484
}
44794485

44804486
void GDScriptAnalyzer::reduce_self(GDScriptParser::SelfNode *p_self) {

modules/gdscript/gdscript_analyzer.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,9 @@ class GDScriptAnalyzer {
145145
void resolve_pending_lambda_bodies();
146146
bool class_exists(const StringName &p_class) const;
147147
void reduce_identifier_from_base_set_class(GDScriptParser::IdentifierNode *p_identifier, GDScriptParser::DataType p_identifier_datatype);
148-
Ref<GDScriptParserRef> ensure_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, const GDScriptParser::ClassNode *p_from_class, const String &p_context, const GDScriptParser::Node *p_source);
149-
Ref<GDScriptParserRef> find_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, const Ref<GDScriptParserRef> &p_dependant_parser);
150-
Ref<GDScriptParserRef> find_cached_parser_for_class(const GDScriptParser::ClassNode *p_class, GDScriptParser *p_dependant_parser);
148+
Ref<GDScriptParserRef> ensure_cached_external_parser_for_class(const GDScriptParser::ClassNode *p_class, const GDScriptParser::ClassNode *p_from_class, const char *p_context, const GDScriptParser::Node *p_source);
149+
Ref<GDScriptParserRef> find_cached_external_parser_for_class(const GDScriptParser::ClassNode *p_class, const Ref<GDScriptParserRef> &p_dependant_parser);
150+
Ref<GDScriptParserRef> find_cached_external_parser_for_class(const GDScriptParser::ClassNode *p_class, GDScriptParser *p_dependant_parser);
151151
Ref<GDScript> get_depended_shallow_script(const String &p_path, Error &r_error);
152152
#ifdef DEBUG_ENABLED
153153
void is_shadowing(GDScriptParser::IdentifierNode *p_identifier, const String &p_context, const bool p_in_local_scope);

modules/gdscript/gdscript_cache.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,10 @@ void GDScriptParserRef::clear() {
135135

136136
GDScriptParserRef::~GDScriptParserRef() {
137137
clear();
138+
138139
if (!abandoned) {
139-
GDScriptCache::remove_parser(path);
140+
MutexLock lock(GDScriptCache::singleton->mutex);
141+
GDScriptCache::singleton->parser_map.erase(path);
140142
}
141143
}
142144

0 commit comments

Comments
 (0)