From 5100eed012eb22f4717a124d23be90fca21d6174 Mon Sep 17 00:00:00 2001 From: Yuri Roubinsky Date: Tue, 29 Jun 2021 14:34:54 +0300 Subject: [PATCH] Added a shader warning about unused local variable --- servers/rendering/shader_language.cpp | 49 +++++++++++++++++++++++++-- servers/rendering/shader_language.h | 5 ++- servers/rendering/shader_warnings.cpp | 4 +++ servers/rendering/shader_warnings.h | 2 ++ 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/servers/rendering/shader_language.cpp b/servers/rendering/shader_language.cpp index 9b2ba36358..f771d6d0ef 100644 --- a/servers/rendering/shader_language.cpp +++ b/servers/rendering/shader_language.cpp @@ -918,6 +918,7 @@ void ShaderLanguage::clear() { used_uniforms.clear(); used_functions.clear(); used_structs.clear(); + used_local_vars.clear(); warnings.clear(); #endif // DEBUG_ENABLED @@ -936,7 +937,7 @@ void ShaderLanguage::clear() { } #ifdef DEBUG_ENABLED -void ShaderLanguage::_parse_used_identifier(const StringName &p_identifier, IdentifierType p_type) { +void ShaderLanguage::_parse_used_identifier(const StringName &p_identifier, IdentifierType p_type, const StringName &p_function) { switch (p_type) { case IdentifierType::IDENTIFIER_CONSTANT: if (HAS_WARNING(ShaderWarning::UNUSED_CONSTANT_FLAG) && used_constants.has(p_identifier)) { @@ -958,6 +959,11 @@ void ShaderLanguage::_parse_used_identifier(const StringName &p_identifier, Iden used_functions[p_identifier].used = true; } break; + case IdentifierType::IDENTIFIER_LOCAL_VAR: + if (HAS_WARNING(ShaderWarning::UNUSED_LOCAL_VARIABLE_FLAG) && used_local_vars.has(p_function) && used_local_vars[p_function].has(p_identifier)) { + used_local_vars[p_function][p_identifier].used = true; + } + break; default: break; } @@ -4171,7 +4177,13 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons expr = func; #ifdef DEBUG_ENABLED if (check_warnings) { - _parse_used_identifier(name, IdentifierType::IDENTIFIER_FUNCTION); + StringName func_name; + + if (p_block && p_block->parent_function) { + func_name = p_block->parent_function->name; + } + + _parse_used_identifier(name, IdentifierType::IDENTIFIER_FUNCTION, func_name); } #endif // DEBUG_ENABLED } @@ -4319,7 +4331,13 @@ ShaderLanguage::Node *ShaderLanguage::_parse_expression(BlockNode *p_block, cons } #ifdef DEBUG_ENABLED if (check_warnings) { - _parse_used_identifier(identifier, ident_type); + StringName func_name; + + if (p_block && p_block->parent_function) { + func_name = p_block->parent_function->name; + } + + _parse_used_identifier(identifier, ident_type, func_name); } #endif // DEBUG_ENABLED } @@ -5537,6 +5555,20 @@ Error ShaderLanguage::_parse_block(BlockNode *p_block, const FunctionInfo &p_fun } } +#ifdef DEBUG_ENABLED + if (check_warnings && HAS_WARNING(ShaderWarning::UNUSED_LOCAL_VARIABLE_FLAG)) { + if (p_block && p_block->parent_function) { + StringName func_name = p_block->parent_function->name; + + if (!used_local_vars.has(func_name)) { + used_local_vars.insert(func_name, Map()); + } + + used_local_vars[func_name].insert(name, Usage(tk_line)); + } + } +#endif // DEBUG_ENABLED + BlockNode::Variable var; var.type = type; var.precision = precision; @@ -7984,6 +8016,15 @@ String ShaderLanguage::get_shader_type(const String &p_code) { #ifdef DEBUG_ENABLED void ShaderLanguage::_check_warning_accums() { + for (Map> *>::Element *E = warnings_check_map2.front(); E; E = E->next()) { + for (Map>::Element *T = (*E->get()).front(); T; T = T->next()) { + for (const Map::Element *U = T->get().front(); U; U = U->next()) { + if (!U->get().used) { + _add_warning(E->key(), U->get().decl_line, U->key()); + } + } + } + } for (Map *>::Element *E = warnings_check_map.front(); E; E = E->next()) { for (const Map::Element *U = (*E->get()).front(); U; U = U->next()) { if (!U->get().used) { @@ -8456,6 +8497,8 @@ ShaderLanguage::ShaderLanguage() { warnings_check_map.insert(ShaderWarning::UNUSED_STRUCT, &used_structs); warnings_check_map.insert(ShaderWarning::UNUSED_UNIFORM, &used_uniforms); warnings_check_map.insert(ShaderWarning::UNUSED_VARYING, &used_varyings); + + warnings_check_map2.insert(ShaderWarning::UNUSED_LOCAL_VARIABLE, &used_local_vars); #endif // DEBUG_ENABLED } diff --git a/servers/rendering/shader_language.h b/servers/rendering/shader_language.h index 4120e04ee1..a91fa57a8e 100644 --- a/servers/rendering/shader_language.h +++ b/servers/rendering/shader_language.h @@ -848,6 +848,9 @@ private: Map used_structs; Map *> warnings_check_map; + Map> used_local_vars; + Map> *> warnings_check_map2; + List warnings; bool check_warnings = false; @@ -917,7 +920,7 @@ private: bool _find_identifier(const BlockNode *p_block, bool p_allow_reassign, const FunctionInfo &p_function_info, const StringName &p_identifier, DataType *r_data_type = nullptr, IdentifierType *r_type = nullptr, bool *r_is_const = nullptr, int *r_array_size = nullptr, StringName *r_struct_name = nullptr, ConstantNode::Value *r_constant_value = nullptr); #ifdef DEBUG_ENABLED - void _parse_used_identifier(const StringName &p_identifier, IdentifierType p_type); + void _parse_used_identifier(const StringName &p_identifier, IdentifierType p_type, const StringName &p_function); #endif // DEBUG_ENABLED bool _is_operator_assign(Operator p_op) const; bool _validate_assign(Node *p_node, const FunctionInfo &p_function_info, String *r_message = nullptr); diff --git a/servers/rendering/shader_warnings.cpp b/servers/rendering/shader_warnings.cpp index aa11b4e397..0c1d6408c9 100644 --- a/servers/rendering/shader_warnings.cpp +++ b/servers/rendering/shader_warnings.cpp @@ -59,6 +59,8 @@ String ShaderWarning::get_message() const { return vformat("The uniform '%s' is declared but never used.", subject); case UNUSED_VARYING: return vformat("The varying '%s' is declared but never used.", subject); + case UNUSED_LOCAL_VARIABLE: + return vformat("The local variable '%s' is declared but never used.", subject); default: break; } @@ -79,6 +81,7 @@ String ShaderWarning::get_name_from_code(Code p_code) { "UNUSED_STRUCT", "UNUSED_UNIFORM", "UNUSED_VARYING", + "UNUSED_LOCAL_VARIABLE", }; static_assert((sizeof(names) / sizeof(*names)) == WARNING_MAX, "Amount of warning types don't match the amount of warning names."); @@ -106,6 +109,7 @@ static void init_code_to_flags_map() { code_to_flags_map->insert(ShaderWarning::UNUSED_STRUCT, ShaderWarning::UNUSED_STRUCT_FLAG); code_to_flags_map->insert(ShaderWarning::UNUSED_UNIFORM, ShaderWarning::UNUSED_UNIFORM_FLAG); code_to_flags_map->insert(ShaderWarning::UNUSED_VARYING, ShaderWarning::UNUSED_VARYING_FLAG); + code_to_flags_map->insert(ShaderWarning::UNUSED_LOCAL_VARIABLE, ShaderWarning::UNUSED_LOCAL_VARIABLE_FLAG); } ShaderWarning::CodeFlags ShaderWarning::get_flags_from_codemap(const Map &p_map) { diff --git a/servers/rendering/shader_warnings.h b/servers/rendering/shader_warnings.h index c40aeefa2d..db872d8fb1 100644 --- a/servers/rendering/shader_warnings.h +++ b/servers/rendering/shader_warnings.h @@ -46,6 +46,7 @@ public: UNUSED_STRUCT, UNUSED_UNIFORM, UNUSED_VARYING, + UNUSED_LOCAL_VARIABLE, WARNING_MAX, }; @@ -57,6 +58,7 @@ public: UNUSED_STRUCT_FLAG = 8U, UNUSED_UNIFORM_FLAG = 16U, UNUSED_VARYING_FLAG = 32U, + UNUSED_LOCAL_VARIABLE_FLAG = 64U, }; private: