Style: Mark clang-format 16 as supported for pre-commit hook #85837

akien-mga · 2023-12-06T12:20:09Z

It only introduced a difference in a .glsl file, which I've worked around by removing an empty line. This keeps formatting consistent between clang-format 15 and 16.

Also added a change in the 3-to-4 project converter to fix bogus formatting in clang-format < 17.

clang-format 17 on the other hand seems to introduce some differences around the handling of * and & in macros, which aren't compatible with the output of earlier versions.

Some of those changes are correct (when * and & are used as math operators), and others are wrong (when they're used as pointer operators).

Below is the full diff generated by clang-format 17. We could live with these changes being the new baseline, even if some are technically a bit wrong. But reapplying clang-format 15 on top of it reverts those changes back, so we'd have to bump our minimal version to 17 :( So for now I'm opting for keeping 13-16 compatibility, and we can evaluate 17+ once clang-format 18 is out.

diff --git a/core/extension/gdextension_interface.cpp b/core/extension/gdextension_interface.cpp
index e02e7aa701..de4875accd 100644
--- a/core/extension/gdextension_interface.cpp
+++ b/core/extension/gdextension_interface.cpp
@@ -1373,7 +1373,7 @@ static void gdextension_editor_remove_plugin(GDExtensionConstStringNamePtr p_cla
 #endif
 }
 
-#define REGISTER_INTERFACE_FUNC(m_name) GDExtension::register_interface_function(#m_name, (GDExtensionInterfaceFunctionPtr)&gdextension_##m_name)
+#define REGISTER_INTERFACE_FUNC(m_name) GDExtension::register_interface_function(#m_name, (GDExtensionInterfaceFunctionPtr) & gdextension_##m_name)
 
 void gdextension_setup_interface() {
 	REGISTER_INTERFACE_FUNC(get_godot_version);
diff --git a/core/math/geometry_3d.cpp b/core/math/geometry_3d.cpp
index 9dd88485f9..1a182e233e 100644
--- a/core/math/geometry_3d.cpp
+++ b/core/math/geometry_3d.cpp
@@ -393,7 +393,7 @@ static inline void _build_faces(uint8_t ***p_cell_status, int x, int y, int z, i
 		return;
 	}
 
-#define vert(m_idx) Vector3(((m_idx)&4) >> 2, ((m_idx)&2) >> 1, (m_idx)&1)
+#define vert(m_idx) Vector3(((m_idx) & 4) >> 2, ((m_idx) & 2) >> 1, (m_idx) & 1)
 
 	static const uint8_t indices[6][4] = {
 		{ 7, 6, 4, 5 },
diff --git a/core/variant/variant_internal.h b/core/variant/variant_internal.h
index 116210e8de..7d6f1c42bf 100644
--- a/core/variant/variant_internal.h
+++ b/core/variant/variant_internal.h
@@ -810,7 +810,7 @@ struct VariantInternalAccessor<bool> {
 #define VARIANT_ACCESSOR_NUMBER(m_type)                                                                        \
 	template <>                                                                                                \
 	struct VariantInternalAccessor<m_type> {                                                                   \
-		static _FORCE_INLINE_ m_type get(const Variant *v) { return (m_type)*VariantInternal::get_int(v); }    \
+		static _FORCE_INLINE_ m_type get(const Variant *v) { return (m_type) * VariantInternal::get_int(v); }  \
 		static _FORCE_INLINE_ void set(Variant *v, m_type p_value) { *VariantInternal::get_int(v) = p_value; } \
 	};
 
diff --git a/core/variant/variant_setget.cpp b/core/variant/variant_setget.cpp
index 05f7abf32c..036ed95ea6 100644
--- a/core/variant/variant_setget.cpp
+++ b/core/variant/variant_setget.cpp
@@ -428,9 +428,9 @@ Variant Variant::get_named(const StringName &p_member, bool &r_valid) const {
 			}                                                                                                                        \
 			m_assign_type num;                                                                                                       \
 			if (value->get_type() == Variant::INT) {                                                                                 \
-				num = (m_assign_type)*VariantGetInternalPtr<int64_t>::get_ptr(value);                                                \
+				num = (m_assign_type) * VariantGetInternalPtr<int64_t>::get_ptr(value);                                              \
 			} else if (value->get_type() == Variant::FLOAT) {                                                                        \
-				num = (m_assign_type)*VariantGetInternalPtr<double>::get_ptr(value);                                                 \
+				num = (m_assign_type) * VariantGetInternalPtr<double>::get_ptr(value);                                               \
 			} else {                                                                                                                 \
 				*oob = false;                                                                                                        \
 				*valid = false;                                                                                                      \
@@ -490,9 +490,9 @@ Variant Variant::get_named(const StringName &p_member, bool &r_valid) const {
 			}                                                                                                                  \
 			m_assign_type num;                                                                                                 \
 			if (value->get_type() == Variant::INT) {                                                                           \
-				num = (m_assign_type)*VariantGetInternalPtr<int64_t>::get_ptr(value);                                          \
+				num = (m_assign_type) * VariantGetInternalPtr<int64_t>::get_ptr(value);                                        \
 			} else if (value->get_type() == Variant::FLOAT) {                                                                  \
-				num = (m_assign_type)*VariantGetInternalPtr<double>::get_ptr(value);                                           \
+				num = (m_assign_type) * VariantGetInternalPtr<double>::get_ptr(value);                                         \
 			} else {                                                                                                           \
 				*oob = false;                                                                                                  \
 				*valid = false;                                                                                                \
diff --git a/drivers/windows/file_access_windows.cpp b/drivers/windows/file_access_windows.cpp
index 9d21073f19..03f6a0f312 100644
--- a/drivers/windows/file_access_windows.cpp
+++ b/drivers/windows/file_access_windows.cpp
@@ -47,7 +47,7 @@
 #include <wchar.h>
 
 #ifdef _MSC_VER
-#define S_ISREG(m) ((m)&_S_IFREG)
+#define S_ISREG(m) ((m) & _S_IFREG)
 #endif
 
 void FileAccessWindows::check_errors() const {
diff --git a/modules/text_server_fb/text_server_fb.cpp b/modules/text_server_fb/text_server_fb.cpp
index f12275d10c..a74c60f981 100644
--- a/modules/text_server_fb/text_server_fb.cpp
+++ b/modules/text_server_fb/text_server_fb.cpp
@@ -73,7 +73,7 @@ using namespace godot;
 
 /*************************************************************************/
 
-#define OT_TAG(c1, c2, c3, c4) ((int32_t)((((uint32_t)(c1)&0xff) << 24) | (((uint32_t)(c2)&0xff) << 16) | (((uint32_t)(c3)&0xff) << 8) | ((uint32_t)(c4)&0xff)))
+#define OT_TAG(c1, c2, c3, c4) ((int32_t)((((uint32_t)(c1) & 0xff) << 24) | (((uint32_t)(c2) & 0xff) << 16) | (((uint32_t)(c3) & 0xff) << 8) | ((uint32_t)(c4) & 0xff)))
 
 bool TextServerFallback::_has_feature(Feature p_feature) const {
 	switch (p_feature) {
diff --git a/platform/android/export/export_plugin.cpp b/platform/android/export/export_plugin.cpp
index f0ee405b41..5859d44679 100644
--- a/platform/android/export/export_plugin.cpp
+++ b/platform/android/export/export_plugin.cpp
@@ -383,7 +383,7 @@ void EditorExportPlatformAndroid::_check_for_changes_poll_thread(void *ud) {
 								d.description += "Chipset: " + p.get_slice("=", 1).strip_edges() + "\n";
 							} else if (p.begins_with("ro.opengles.version=")) {
 								uint32_t opengl = p.get_slice("=", 1).to_int();
-								d.description += "OpenGL: " + itos(opengl >> 16) + "." + itos((opengl >> 8) & 0xFF) + "." + itos((opengl)&0xFF) + "\n";
+								d.description += "OpenGL: " + itos(opengl >> 16) + "." + itos((opengl >> 8) & 0xFF) + "." + itos((opengl) & 0xFF) + "\n";
 							}
 						}
 
diff --git a/platform/macos/display_server_macos.mm b/platform/macos/display_server_macos.mm
index 407a315827..d2f28ea61e 100644
--- a/platform/macos/display_server_macos.mm
+++ b/platform/macos/display_server_macos.mm
@@ -3964,7 +3964,7 @@ void DisplayServerMacOS::cursor_set_custom_image(const Ref<Resource> &p_cursor,
 			uint8_t alpha = (color >> 24) & 0xFF;
 			pixels[i * 4 + 0] = ((color >> 16) & 0xFF) * alpha / 255;
 			pixels[i * 4 + 1] = ((color >> 8) & 0xFF) * alpha / 255;
-			pixels[i * 4 + 2] = ((color)&0xFF) * alpha / 255;
+			pixels[i * 4 + 2] = ((color) & 0xFF) * alpha / 255;
 			pixels[i * 4 + 3] = alpha;
 		}
 
diff --git a/platform/windows/display_server_windows.cpp b/platform/windows/display_server_windows.cpp
index c801ca96e7..68259ee5ab 100644
--- a/platform/windows/display_server_windows.cpp
+++ b/platform/windows/display_server_windows.cpp
@@ -2627,9 +2627,9 @@ void DisplayServerWindows::set_context(Context p_context) {
 #define SIGNATURE_MASK 0xFFFFFF00
 // Keeping the name suggested by Microsoft, but this macro really answers:
 // Is this mouse event emulated from touch or pen input?
-#define IsPenEvent(dw) (((dw)&SIGNATURE_MASK) == MI_WP_SIGNATURE)
+#define IsPenEvent(dw) (((dw) & SIGNATURE_MASK) == MI_WP_SIGNATURE)
 // This one tells whether the event comes from touchscreen (and not from pen).
-#define IsTouchEvent(dw) (IsPenEvent(dw) && ((dw)&0x80))
+#define IsTouchEvent(dw) (IsPenEvent(dw) && ((dw) & 0x80))
 
 void DisplayServerWindows::_touch_event(WindowID p_window, bool p_pressed, float p_x, float p_y, int idx) {
 	if (touch_state.has(idx) == p_pressed) {
diff --git a/servers/physics_3d/godot_soft_body_3d.cpp b/servers/physics_3d/godot_soft_body_3d.cpp
index 0c2b935554..e621977326 100644
--- a/servers/physics_3d/godot_soft_body_3d.cpp
+++ b/servers/physics_3d/godot_soft_body_3d.cpp
@@ -595,7 +595,7 @@ void GodotSoftBody3D::generate_bending_constraints(int p_distance) {
 		const uint32_t adj_size = n * n;
 		unsigned *adj = memnew_arr(unsigned, adj_size);
 
-#define IDX(_x_, _y_) ((_y_)*n + (_x_))
+#define IDX(_x_, _y_) ((_y_) * n + (_x_))
 		for (j = 0; j < n; ++j) {
 			for (i = 0; i < n; ++i) {
 				int idx_ij = j * n + i;
diff --git a/tests/test_macros.h b/tests/test_macros.h
index bc85ec6ddc..a173b37a2d 100644
--- a/tests/test_macros.h
+++ b/tests/test_macros.h
@@ -161,11 +161,11 @@ int register_test_command(String p_command, TestFunc p_function);
 		MessageQueue::get_singleton()->flush();                              \
 	}
 
-#define _UPDATE_EVENT_MODIFERS(m_event, m_modifers)                                 \
-	m_event->set_shift_pressed(((m_modifers)&KeyModifierMask::SHIFT) != Key::NONE); \
-	m_event->set_alt_pressed(((m_modifers)&KeyModifierMask::ALT) != Key::NONE);     \
-	m_event->set_ctrl_pressed(((m_modifers)&KeyModifierMask::CTRL) != Key::NONE);   \
-	m_event->set_meta_pressed(((m_modifers)&KeyModifierMask::META) != Key::NONE);
+#define _UPDATE_EVENT_MODIFERS(m_event, m_modifers)                                   \
+	m_event->set_shift_pressed(((m_modifers) & KeyModifierMask::SHIFT) != Key::NONE); \
+	m_event->set_alt_pressed(((m_modifers) & KeyModifierMask::ALT) != Key::NONE);     \
+	m_event->set_ctrl_pressed(((m_modifers) & KeyModifierMask::CTRL) != Key::NONE);   \
+	m_event->set_meta_pressed(((m_modifers) & KeyModifierMask::META) != Key::NONE);
 
 #define _CREATE_GUI_MOUSE_EVENT(m_screen_pos, m_input, m_mask, m_modifers) \
 	Ref<InputEventMouseButton> event;                                      \

It only introduced a difference in a .glsl file, which I've worked around by removing an empty line. This keeps formatting consistent between clang-format 15 and 16. Also added a change in the 3-to-4 project converter to fix bogus formatting in clang-format < 17.

YuriSizov · 2024-01-24T15:08:19Z

Cherry-picked for 4.2.2.

YuriSizov · 2024-01-24T17:21:25Z

Cherry-picked for 4.1.4.

akien-mga · 2024-01-30T13:43:26Z

Cherry-picked for 3.6.
Cherry-picked for 3.5.4.

akien-mga requested review from a team as code owners December 6, 2023 12:20

akien-mga added this to the 4.3 milestone Dec 6, 2023

akien-mga force-pushed the clang-format-16 branch from 73793c3 to 49f4860 Compare December 6, 2023 12:27

akien-mga merged commit e72e63a into godotengine:master Jan 5, 2024
15 checks passed

akien-mga deleted the clang-format-16 branch January 5, 2024 11:50

YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Jan 24, 2024

YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jan 24, 2024

akien-mga removed cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Jan 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Style: Mark clang-format 16 as supported for pre-commit hook #85837

Style: Mark clang-format 16 as supported for pre-commit hook #85837

akien-mga commented Dec 6, 2023 •

edited

Loading

YuriSizov commented Jan 24, 2024

YuriSizov commented Jan 24, 2024

akien-mga commented Jan 30, 2024

Style: Mark clang-format 16 as supported for pre-commit hook #85837

Style: Mark clang-format 16 as supported for pre-commit hook #85837

Conversation

akien-mga commented Dec 6, 2023 • edited Loading

YuriSizov commented Jan 24, 2024

YuriSizov commented Jan 24, 2024

akien-mga commented Jan 30, 2024

akien-mga commented Dec 6, 2023 •

edited

Loading