From fc370b3feb419fdc1a8139bdf01f1dacf868ca1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Fri, 5 Apr 2019 14:06:16 +0200 Subject: [PATCH] Fix -Wimplicit-fallthrough warnings from GCC 8 Adds `FALLTHROUGH` macro to specify when a fallthrough is intentional. Can be replaced by `[[fallthrough]]` if/when we switch to C++17. The warning is now enabled by default for GCC on `extra` warnings level (part of GCC's `-Wextra`). It's not enabled in Clang's `-Wextra` yet, but we could enable it manually once we switch to C++11. There's no equivalent feature in MSVC for now. Fixes #26135. --- SConstruct | 7 +-- core/io/multiplayer_api.cpp | 3 +- core/typedefs.h | 23 ++++++++- editor/editor_node.cpp | 7 +-- editor/plugins/canvas_item_editor_plugin.cpp | 5 ++ editor/plugins/script_editor_plugin.cpp | 3 +- editor/plugins/script_text_editor.cpp | 2 +- editor/project_manager.cpp | 2 +- main/tests/test_gdscript.cpp | 1 + modules/gdscript/gdscript_parser.cpp | 18 ++++--- modules/gdscript/gdscript_tokenizer.cpp | 2 +- modules/mono/editor/bindings_generator.cpp | 1 + scene/2d/collision_object_2d.cpp | 7 +-- scene/3d/collision_object.cpp | 2 +- scene/gui/button.cpp | 2 + scene/gui/line_edit.cpp | 9 ++-- scene/gui/text_edit.cpp | 51 +++++--------------- scene/gui/tree.cpp | 4 +- 18 files changed, 82 insertions(+), 67 deletions(-) diff --git a/SConstruct b/SConstruct index 4a97cb9407..1271156a28 100644 --- a/SConstruct +++ b/SConstruct @@ -346,13 +346,14 @@ if selected_platform in platform_list: version = methods.get_compiler_version(env) if version != None and version[0] >= '7': shadow_local_warning = ['-Wshadow-local'] + if (env["warnings"] == 'extra'): - # FIXME: enable -Wimplicit-fallthrough once #26135 is fixed # FIXME: enable -Wclobbered once #26351 is fixed - env.Append(CCFLAGS=['-Wall', '-Wextra', '-Wno-implicit-fallthrough', '-Wno-unused-parameter'] + all_plus_warnings + shadow_local_warning) + # Note: enable -Wimplicit-fallthrough for Clang (already part of -Wextra for GCC) + # once we switch to C++11 or later (necessary for our FALLTHROUGH macro). + env.Append(CCFLAGS=['-Wall', '-Wextra', '-Wno-unused-parameter'] + all_plus_warnings + shadow_local_warning) if methods.using_gcc(env): env['CCFLAGS'] += ['-Wno-clobbered'] - elif (env["warnings"] == 'all'): env.Append(CCFLAGS=['-Wall'] + shadow_local_warning) elif (env["warnings"] == 'moderate'): diff --git a/core/io/multiplayer_api.cpp b/core/io/multiplayer_api.cpp index 22530ad721..f74f076c65 100644 --- a/core/io/multiplayer_api.cpp +++ b/core/io/multiplayer_api.cpp @@ -46,7 +46,8 @@ _FORCE_INLINE_ bool _should_call_local(MultiplayerAPI::RPCMode mode, bool is_mas case MultiplayerAPI::RPC_MODE_MASTERSYNC: { if (is_master) r_skip_rpc = true; // I am the master, so skip remote call. - } // Do not break, fall over to other sync. + FALLTHROUGH; + } case MultiplayerAPI::RPC_MODE_REMOTESYNC: case MultiplayerAPI::RPC_MODE_PUPPETSYNC: { // Call it, sync always results in a local call. diff --git a/core/typedefs.h b/core/typedefs.h index a3d7314b1c..660139b90a 100644 --- a/core/typedefs.h +++ b/core/typedefs.h @@ -331,7 +331,28 @@ struct _GlobalLock { /** This is needed due to a strange OpenGL API that expects a pointer * type for an argument that is actually an offset. */ - #define CAST_INT_TO_UCHAR_PTR(ptr) ((uint8_t *)(uintptr_t)(ptr)) +/** Hint for compilers that this fallthrough in a switch is intentional. + * Can be replaced by [[fallthrough]] annotation if we move to C++17. + * Including conditional support for it for people who set -std=c++17 + * themselves. + * Requires a trailing semicolon when used. + */ +#if __cplusplus >= 201703L +#define FALLTHROUGH [[fallthrough]] +#elif defined(__GNUC__) && __GNUC__ >= 7 +#define FALLTHROUGH __attribute__((fallthrough)) +#elif defined(__llvm__) && __cplusplus >= 201103L && defined(__has_feature) +#if __has_feature(cxx_attributes) && defined(__has_warning) +#if __has_warning("-Wimplicit-fallthrough") +#define FALLTHROUGH [[clang::fallthrough]] +#endif +#endif +#endif + +#ifndef FALLTHROUGH +#define FALLTHROUGH +#endif + #endif // TYPEDEFS_H diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp index 2f03a9943f..6c629c1869 100644 --- a/editor/editor_node.cpp +++ b/editor/editor_node.cpp @@ -1900,7 +1900,8 @@ void EditorNode::_menu_option_confirm(int p_option, bool p_confirmed) { break; } - } // fallthrough + FALLTHROUGH; + } case SCENE_TAB_CLOSE: case FILE_SAVE_SCENE: { @@ -1919,8 +1920,8 @@ void EditorNode::_menu_option_confirm(int p_option, bool p_confirmed) { break; } - // fallthrough to save_as - }; + FALLTHROUGH; + } case FILE_SAVE_AS_SCENE: { int scene_idx = (p_option == FILE_SAVE_SCENE || p_option == FILE_SAVE_AS_SCENE) ? -1 : tab_closing; diff --git a/editor/plugins/canvas_item_editor_plugin.cpp b/editor/plugins/canvas_item_editor_plugin.cpp index dc1a8ade9f..e0d0c18fd2 100644 --- a/editor/plugins/canvas_item_editor_plugin.cpp +++ b/editor/plugins/canvas_item_editor_plugin.cpp @@ -2261,6 +2261,7 @@ void CanvasItemEditor::_gui_input_viewport(const Ref &p_event) { break; case DRAG_PAN: c = CURSOR_DRAG; + break; default: break; } @@ -2597,6 +2598,7 @@ void CanvasItemEditor::_draw_control_helpers(Control *control) { case DRAG_TOP_LEFT: case DRAG_BOTTOM_LEFT: _draw_margin_at_position(control->get_size().width, parent_transform.xform(Vector2((node_pos_in_parent[0] + node_pos_in_parent[2]) / 2, node_pos_in_parent[3])) + Vector2(0, 5), MARGIN_BOTTOM); + FALLTHROUGH; case DRAG_MOVE: start = Vector2(node_pos_in_parent[0], Math::lerp(node_pos_in_parent[1], node_pos_in_parent[3], ratio)); end = start - Vector2(control->get_margin(MARGIN_LEFT), 0); @@ -2611,6 +2613,7 @@ void CanvasItemEditor::_draw_control_helpers(Control *control) { case DRAG_TOP_RIGHT: case DRAG_BOTTOM_RIGHT: _draw_margin_at_position(control->get_size().width, parent_transform.xform(Vector2((node_pos_in_parent[0] + node_pos_in_parent[2]) / 2, node_pos_in_parent[3])) + Vector2(0, 5), MARGIN_BOTTOM); + FALLTHROUGH; case DRAG_MOVE: start = Vector2(node_pos_in_parent[2], Math::lerp(node_pos_in_parent[3], node_pos_in_parent[1], ratio)); end = start - Vector2(control->get_margin(MARGIN_RIGHT), 0); @@ -2625,6 +2628,7 @@ void CanvasItemEditor::_draw_control_helpers(Control *control) { case DRAG_TOP_LEFT: case DRAG_TOP_RIGHT: _draw_margin_at_position(control->get_size().height, parent_transform.xform(Vector2(node_pos_in_parent[2], (node_pos_in_parent[1] + node_pos_in_parent[3]) / 2)) + Vector2(5, 0), MARGIN_RIGHT); + FALLTHROUGH; case DRAG_MOVE: start = Vector2(Math::lerp(node_pos_in_parent[0], node_pos_in_parent[2], ratio), node_pos_in_parent[1]); end = start - Vector2(0, control->get_margin(MARGIN_TOP)); @@ -2639,6 +2643,7 @@ void CanvasItemEditor::_draw_control_helpers(Control *control) { case DRAG_BOTTOM_LEFT: case DRAG_BOTTOM_RIGHT: _draw_margin_at_position(control->get_size().height, parent_transform.xform(Vector2(node_pos_in_parent[2], (node_pos_in_parent[1] + node_pos_in_parent[3]) / 2) + Vector2(5, 0)), MARGIN_RIGHT); + FALLTHROUGH; case DRAG_MOVE: start = Vector2(Math::lerp(node_pos_in_parent[2], node_pos_in_parent[0], ratio), node_pos_in_parent[3]); end = start - Vector2(0, control->get_margin(MARGIN_BOTTOM)); diff --git a/editor/plugins/script_editor_plugin.cpp b/editor/plugins/script_editor_plugin.cpp index 89e3327a3a..3a61277f36 100644 --- a/editor/plugins/script_editor_plugin.cpp +++ b/editor/plugins/script_editor_plugin.cpp @@ -849,8 +849,7 @@ void ScriptEditor::_file_dialog_action(String p_file) { } file->close(); memdelete(file); - - // fallthrough to open the file. + FALLTHROUGH; } case FILE_OPEN: { diff --git a/editor/plugins/script_text_editor.cpp b/editor/plugins/script_text_editor.cpp index 8c3f8fd3e8..9fc42e3862 100644 --- a/editor/plugins/script_text_editor.cpp +++ b/editor/plugins/script_text_editor.cpp @@ -910,7 +910,7 @@ void ScriptTextEditor::_edit_option(int p_op) { tx->set_line_as_breakpoint(line, dobreak); ScriptEditor::get_singleton()->get_debugger()->set_breakpoint(script->get_path(), line + 1, dobreak); } - } + } break; case DEBUG_GOTO_NEXT_BREAKPOINT: { List bpoints; diff --git a/editor/project_manager.cpp b/editor/project_manager.cpp index a3bcfa6e70..095ec891cd 100644 --- a/editor/project_manager.cpp +++ b/editor/project_manager.cpp @@ -1136,7 +1136,7 @@ void ProjectManager::_unhandled_input(const Ref &p_ev) { break; } - // else fallthrough to key_down + FALLTHROUGH; } case KEY_DOWN: { diff --git a/main/tests/test_gdscript.cpp b/main/tests/test_gdscript.cpp index 27180f84aa..7a711e8cc9 100644 --- a/main/tests/test_gdscript.cpp +++ b/main/tests/test_gdscript.cpp @@ -127,6 +127,7 @@ static String _parser_expr(const GDScriptParser::Node *p_expr) { case GDScriptParser::OperatorNode::OP_PARENT_CALL: txt += "."; + FALLTHROUGH; case GDScriptParser::OperatorNode::OP_CALL: { ERR_FAIL_COND_V(c_node->arguments.size() < 1, ""); diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 6b7379d350..e75b8a14a3 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -777,7 +777,8 @@ GDScriptParser::Node *GDScriptParser::_parse_expression(Node *p_parent, bool p_s } _add_warning(GDScriptWarning::UNASSIGNED_VARIABLE_OP_ASSIGN, -1, identifier.operator String()); } - } // fallthrough + FALLTHROUGH; + } case GDScriptTokenizer::TK_OP_ASSIGN: { lv->assignments += 1; lv->usages--; // Assignment is not really usage @@ -3636,7 +3637,8 @@ void GDScriptParser::_parse_class(ClassNode *p_class) { return; } - }; //fallthrough to function + FALLTHROUGH; + } case GDScriptTokenizer::TK_PR_FUNCTION: { bool _static = false; @@ -4086,7 +4088,8 @@ void GDScriptParser::_parse_class(ClassNode *p_class) { break; } - }; //fallthrough to use the same + FALLTHROUGH; + } case Variant::REAL: { if (tokenizer->get_token() == GDScriptTokenizer::TK_IDENTIFIER && tokenizer->get_token_identifier() == "EASE") { @@ -4511,6 +4514,7 @@ void GDScriptParser::_parse_class(ClassNode *p_class) { #ifdef DEBUG_ENABLED _add_warning(GDScriptWarning::DEPRECATED_KEYWORD, tokenizer->get_token_line(), "slave", "puppet"); #endif + FALLTHROUGH; case GDScriptTokenizer::TK_PR_PUPPET: { //may be fallthrough from export, ignore if so @@ -4578,7 +4582,7 @@ void GDScriptParser::_parse_class(ClassNode *p_class) { continue; } break; case GDScriptTokenizer::TK_PR_VAR: { - //variale declaration and (eventual) initialization + // variable declaration and (eventual) initialization ClassNode::Member member; @@ -5290,7 +5294,8 @@ String GDScriptParser::DataType::to_string() const { if (!gds_class.empty()) { return gds_class; } - } // fallthrough + FALLTHROUGH; + } case SCRIPT: { if (is_meta_type) { return script_type->get_class_name().operator String(); @@ -8046,7 +8051,8 @@ void GDScriptParser::_check_block_types(BlockNode *p_block) { if (cn->value.get_type() == Variant::STRING) { break; } - } // falthrough + FALLTHROUGH; + } default: { _mark_line_as_safe(statement->line); _reduce_node_type(statement); // Test for safety anyway diff --git a/modules/gdscript/gdscript_tokenizer.cpp b/modules/gdscript/gdscript_tokenizer.cpp index 8962e3bb34..abc739d645 100644 --- a/modules/gdscript/gdscript_tokenizer.cpp +++ b/modules/gdscript/gdscript_tokenizer.cpp @@ -744,7 +744,7 @@ void GDScriptTokenizerText::_advance() { } INCPOS(1); is_node_path = true; - + FALLTHROUGH; case '\'': case '"': { diff --git a/modules/mono/editor/bindings_generator.cpp b/modules/mono/editor/bindings_generator.cpp index 35a89956a8..b39e8bb568 100644 --- a/modules/mono/editor/bindings_generator.cpp +++ b/modules/mono/editor/bindings_generator.cpp @@ -2406,6 +2406,7 @@ void BindingsGenerator::_default_argument_from_variant(const Variant &p_val, Arg r_iarg.default_argument = "null"; break; } + FALLTHROUGH; case Variant::DICTIONARY: case Variant::_RID: r_iarg.default_argument = "new %s()"; diff --git a/scene/2d/collision_object_2d.cpp b/scene/2d/collision_object_2d.cpp index d54070df8d..f43d97eb2a 100644 --- a/scene/2d/collision_object_2d.cpp +++ b/scene/2d/collision_object_2d.cpp @@ -29,6 +29,7 @@ /*************************************************************************/ #include "collision_object_2d.h" + #include "scene/scene_string_names.h" #include "servers/physics_2d_server.h" @@ -56,7 +57,7 @@ void CollisionObject2D::_notification(int p_what) { _update_pickable(); //get space - } + } break; case NOTIFICATION_ENTER_CANVAS: { @@ -64,7 +65,7 @@ void CollisionObject2D::_notification(int p_what) { Physics2DServer::get_singleton()->area_attach_canvas_instance_id(rid, get_canvas_layer_instance_id()); else Physics2DServer::get_singleton()->body_attach_canvas_instance_id(rid, get_canvas_layer_instance_id()); - } + } break; case NOTIFICATION_VISIBILITY_CHANGED: { @@ -101,7 +102,7 @@ void CollisionObject2D::_notification(int p_what) { Physics2DServer::get_singleton()->area_attach_canvas_instance_id(rid, 0); else Physics2DServer::get_singleton()->body_attach_canvas_instance_id(rid, 0); - } + } break; } } diff --git a/scene/3d/collision_object.cpp b/scene/3d/collision_object.cpp index d8c2042c88..f542b021be 100644 --- a/scene/3d/collision_object.cpp +++ b/scene/3d/collision_object.cpp @@ -52,7 +52,7 @@ void CollisionObject::_notification(int p_what) { _update_pickable(); //get space - }; + } break; case NOTIFICATION_TRANSFORM_CHANGED: { diff --git a/scene/gui/button.cpp b/scene/gui/button.cpp index c734136895..65e9cccd05 100644 --- a/scene/gui/button.cpp +++ b/scene/gui/button.cpp @@ -29,6 +29,7 @@ /*************************************************************************/ #include "button.h" + #include "core/translation.h" #include "servers/visual_server.h" @@ -102,6 +103,7 @@ void Button::_notification(int p_what) { break; } + FALLTHROUGH; } case DRAW_PRESSED: { diff --git a/scene/gui/line_edit.cpp b/scene/gui/line_edit.cpp index 874246586d..0c5dc39273 100644 --- a/scene/gui/line_edit.cpp +++ b/scene/gui/line_edit.cpp @@ -29,6 +29,7 @@ /*************************************************************************/ #include "line_edit.h" + #include "core/message_queue.h" #include "core/os/keyboard.h" #include "core/os/os.h" @@ -320,7 +321,7 @@ void LineEdit::_gui_input(Ref p_event) { handled = false; break; } - // numlock disabled. fallthrough to key_left + FALLTHROUGH; } case KEY_LEFT: { @@ -367,7 +368,7 @@ void LineEdit::_gui_input(Ref p_event) { handled = false; break; } - // numlock disabled. fallthrough to key_right + FALLTHROUGH; } case KEY_RIGHT: { @@ -474,7 +475,7 @@ void LineEdit::_gui_input(Ref p_event) { handled = false; break; } - // numlock disabled. fallthrough to key_home + FALLTHROUGH; } case KEY_HOME: { @@ -487,7 +488,7 @@ void LineEdit::_gui_input(Ref p_event) { handled = false; break; } - // numlock disabled. fallthrough to key_end + FALLTHROUGH; } case KEY_END: { diff --git a/scene/gui/text_edit.cpp b/scene/gui/text_edit.cpp index 2bf2364873..9acf8d40c9 100644 --- a/scene/gui/text_edit.cpp +++ b/scene/gui/text_edit.cpp @@ -2517,7 +2517,7 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { scancode_handled = false; break; } - // numlock disabled. fallthrough to key_left + FALLTHROUGH; } case KEY_LEFT: { @@ -2580,7 +2580,7 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { scancode_handled = false; break; } - // numlock disabled. fallthrough to key_right + FALLTHROUGH; } case KEY_RIGHT: { @@ -2641,7 +2641,7 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { scancode_handled = false; break; } - // numlock disabled. fallthrough to key_up + FALLTHROUGH; } case KEY_UP: { @@ -2694,7 +2694,7 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { scancode_handled = false; break; } - // numlock disabled. fallthrough to key_down + FALLTHROUGH; } case KEY_DOWN: { @@ -2817,11 +2817,10 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { scancode_handled = false; break; } - // numlock disabled. fallthrough to key_home + FALLTHROUGH; } -#ifdef APPLE_STYLE_KEYS case KEY_HOME: { - +#ifdef APPLE_STYLE_KEYS if (k->get_shift()) _pre_shift_selection(); @@ -2831,11 +2830,7 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { _post_shift_selection(); else if (k->get_command() || k->get_control()) deselect(); - - } break; #else - case KEY_HOME: { - if (k->get_shift()) _pre_shift_selection(); @@ -2876,19 +2871,17 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { deselect(); _cancel_completion(); completion_hint = ""; - - } break; #endif + } break; case KEY_KP_1: { if (k->get_unicode() != 0) { scancode_handled = false; break; } - // numlock disabled. fallthrough to key_end + FALLTHROUGH; } -#ifdef APPLE_STYLE_KEYS case KEY_END: { - +#ifdef APPLE_STYLE_KEYS if (k->get_shift()) _pre_shift_selection(); @@ -2898,11 +2891,7 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { _post_shift_selection(); else if (k->get_command() || k->get_control()) deselect(); - - } break; #else - case KEY_END: { - if (k->get_shift()) _pre_shift_selection(); @@ -2929,15 +2918,14 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { _cancel_completion(); completion_hint = ""; - - } break; #endif + } break; case KEY_KP_9: { if (k->get_unicode() != 0) { scancode_handled = false; break; } - // numlock disabled. fallthrough to key_pageup + FALLTHROUGH; } case KEY_PAGEUP: { @@ -2960,7 +2948,7 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { scancode_handled = false; break; } - // numlock disabled. fallthrough to key_pagedown + FALLTHROUGH; } case KEY_PAGEDOWN: { @@ -3139,21 +3127,7 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { if (scancode_handled) accept_event(); - /* - if (!scancode_handled && !k->get_command() && !k->get_alt()) { - if (k->get_unicode()>=32) { - - if (readonly) - break; - - accept_event(); - } else { - - break; - } - } -*/ if (k->get_scancode() == KEY_INSERT) { set_insert_mode(!insert_mode); accept_event(); @@ -3196,7 +3170,6 @@ void TextEdit::_gui_input(const Ref &p_gui_input) { end_complex_operation(); } accept_event(); - } else { } } diff --git a/scene/gui/tree.cpp b/scene/gui/tree.cpp index 49cad6fccf..679b752fa3 100644 --- a/scene/gui/tree.cpp +++ b/scene/gui/tree.cpp @@ -29,7 +29,6 @@ /*************************************************************************/ #include "tree.h" -#include #include "core/math/math_funcs.h" #include "core/os/input.h" @@ -43,6 +42,8 @@ #include "editor/editor_node.h" #endif +#include + void TreeItem::move_to_top() { if (!parent || parent->children == this) @@ -940,6 +941,7 @@ int Tree::compute_item_height(TreeItem *p_item) const { int check_icon_h = cache.checked->get_height(); if (height < check_icon_h) height = check_icon_h; + FALLTHROUGH; } case TreeItem::CELL_MODE_STRING: case TreeItem::CELL_MODE_CUSTOM: