Merge pull request #27677 from akien-mga/Wimplicit-fallthrough

Fix -Wimplicit-fallthrough warnings from GCC 8

SConstruct

@@ -342,17 +342,18 @@ if selected_platform in platform_list:
         shadow_local_warning = []
         all_plus_warnings = ['-Wwrite-strings']
 
-        if methods.use_gcc(env):
+        if methods.using_gcc(env):
             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)
-            if methods.use_gcc(env):
-                env['CCFLAGS'] += ['-Wno-clobbered']
 
+        if (env["warnings"] == 'extra'):
+            # FIXME: enable -Wclobbered once #26351 is fixed
+            # 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'):

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.

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

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;
 

editor/plugins/canvas_item_editor_plugin.cpp

@@ -2261,6 +2261,7 @@ void CanvasItemEditor::_gui_input_viewport(const Ref<InputEvent> &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));

editor/plugins/script_editor_plugin.cpp

@@ -849,8 +849,7 @@ void ScriptEditor::_file_dialog_action(String p_file) {
 			}
 			file->close();
 			memdelete(file);
-
+			FALLTHROUGH;
-			// fallthrough to open the file.
 		}
 		case FILE_OPEN: {
 

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<int> bpoints;

editor/project_manager.cpp

@@ -1136,7 +1136,7 @@ void ProjectManager::_unhandled_input(const Ref<InputEvent> &p_ev) {
 
 					break;
 				}
-				// else fallthrough to key_down
+				FALLTHROUGH;
 			}
 			case KEY_DOWN: {
 

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, "");

methods.py

@@ -668,5 +668,8 @@ def get_compiler_version(env):
     else:
         return None
 
-def use_gcc(env):
+def using_gcc(env):
     return 'gcc' in os.path.basename(env["CC"])
+
+def using_clang(env):
+    return 'clang' in os.path.basename(env["CC"])

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

modules/gdscript/gdscript_tokenizer.cpp

@@ -744,7 +744,7 @@ void GDScriptTokenizerText::_advance() {
 				}
 				INCPOS(1);
 				is_node_path = true;
-
+				FALLTHROUGH;
 			case '\'':
 			case '"': {
 

modules/mono/editor/bindings_generator.cpp

@@ -2497,6 +2497,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()";

platform/server/detect.py

@@ -1,7 +1,6 @@
 import os
 import platform
 import sys
-from methods import get_compiler_version, use_gcc
 
 # This file is mostly based on platform/x11/detect.py.
 # If editing this file, make sure to apply relevant changes here too.

platform/x11/detect.py

@@ -2,7 +2,7 @@ import os
 import platform
 import sys
 from compat import decode_utf8
-from methods import get_compiler_version, use_gcc
+from methods import get_compiler_version, using_gcc
 
 def is_active():
     return True
@@ -160,7 +160,7 @@ def configure(env):
     env.Append(LINKFLAGS=['-pipe'])
 
     # Check for gcc version >= 6 before adding -no-pie
-    if use_gcc(env):
+    if using_gcc(env):
         version = get_compiler_version(env)
         if version != None and version[0] >= '6':
             env.Append(CCFLAGS=['-fpie'])

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;
 	}
 }
 

scene/3d/collision_object.cpp

@@ -52,7 +52,7 @@ void CollisionObject::_notification(int p_what) {
 
 			_update_pickable();
 			//get space
-		};
+		} break;
 
 		case NOTIFICATION_TRANSFORM_CHANGED: {
 

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: {
 

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<InputEvent> p_event) {
 						handled = false;
 						break;
 					}
-					// numlock disabled. fallthrough to key_left
+					FALLTHROUGH;
 				}
 				case KEY_LEFT: {
 
@@ -367,7 +368,7 @@ void LineEdit::_gui_input(Ref<InputEvent> p_event) {
 						handled = false;
 						break;
 					}
-					// numlock disabled. fallthrough to key_right
+					FALLTHROUGH;
 				}
 				case KEY_RIGHT: {
 
@@ -474,7 +475,7 @@ void LineEdit::_gui_input(Ref<InputEvent> p_event) {
 						handled = false;
 						break;
 					}
-					// numlock disabled. fallthrough to key_home
+					FALLTHROUGH;
 				}
 				case KEY_HOME: {
 
@@ -487,7 +488,7 @@ void LineEdit::_gui_input(Ref<InputEvent> p_event) {
 						handled = false;
 						break;
 					}
-					// numlock disabled. fallthrough to key_end
+					FALLTHROUGH;
 				}
 				case KEY_END: {
 

scene/gui/text_edit.cpp

@@ -2517,7 +2517,7 @@ void TextEdit::_gui_input(const Ref<InputEvent> &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<InputEvent> &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<InputEvent> &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<InputEvent> &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<InputEvent> &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<InputEvent> &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<InputEvent> &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<InputEvent> &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<InputEvent> &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<InputEvent> &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<InputEvent> &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<InputEvent> &p_gui_input) {
 					end_complex_operation();
 				}
 				accept_event();
-			} else {
 			}
 		}
 

scene/gui/tree.cpp

@@ -29,7 +29,6 @@
 /*************************************************************************/
 
 #include "tree.h"
-#include <limits.h>
 
 #include "core/math/math_funcs.h"
 #include "core/os/input.h"
@@ -43,6 +42,8 @@
 #include "editor/editor_node.h"
 #endif
 
+#include <limits.h>
+
 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: