From adf14bfddefdbbfe70cbb2bb000c6a9a4c19e4e0 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Thu, 20 Jan 2022 12:51:13 +0000 Subject: [PATCH] Add nodiscard to core math classes to catch c++ errors. A common source of errors is to call functions (such as round()) expecting them to work in place, but them actually being designed only to return the processed value. Not using the return value in this case in indicative of a bug, and can be flagged as a warning by using the [[nodiscard]] attribute. --- core/color.h | 2 +- core/math/aabb.h | 2 +- core/math/basis.h | 2 +- core/math/face3.h | 2 +- core/math/plane.h | 2 +- core/math/quat.h | 4 +- core/math/rect2.h | 4 +- core/math/transform.h | 2 +- core/math/transform_2d.h | 2 +- core/math/vector2.h | 4 +- core/math/vector3.h | 2 +- core/typedefs.h | 41 ++++++++++++++++++++ modules/bullet/cone_twist_joint_bullet.cpp | 4 +- modules/bullet/generic_6dof_joint_bullet.cpp | 4 +- modules/bullet/hinge_joint_bullet.cpp | 4 +- modules/bullet/slider_joint_bullet.cpp | 4 +- platform/osx/os_osx.mm | 6 +-- scene/2d/canvas_item.cpp | 2 +- 18 files changed, 67 insertions(+), 26 deletions(-) diff --git a/core/color.h b/core/color.h index 55f62109f7b..c08394e09e8 100644 --- a/core/color.h +++ b/core/color.h @@ -34,7 +34,7 @@ #include "core/math/math_funcs.h" #include "core/ustring.h" -struct Color { +struct _NO_DISCARD_CLASS_ Color { union { struct { float r; diff --git a/core/math/aabb.h b/core/math/aabb.h index a6452808d99..f973bd9c7e1 100644 --- a/core/math/aabb.h +++ b/core/math/aabb.h @@ -40,7 +40,7 @@ * This is implemented by a point (position) and the box size */ -class AABB { +class _NO_DISCARD_CLASS_ AABB { public: Vector3 position; Vector3 size; diff --git a/core/math/basis.h b/core/math/basis.h index 4e3388102a7..020c759f898 100644 --- a/core/math/basis.h +++ b/core/math/basis.h @@ -34,7 +34,7 @@ #include "core/math/quat.h" #include "core/math/vector3.h" -class Basis { +class _NO_DISCARD_CLASS_ Basis { public: Vector3 elements[3] = { Vector3(1, 0, 0), diff --git a/core/math/face3.h b/core/math/face3.h index f3429e2be5c..2197f81a71b 100644 --- a/core/math/face3.h +++ b/core/math/face3.h @@ -36,7 +36,7 @@ #include "core/math/transform.h" #include "core/math/vector3.h" -class Face3 { +class _NO_DISCARD_CLASS_ Face3 { public: enum Side { SIDE_OVER, diff --git a/core/math/plane.h b/core/math/plane.h index 659ec328419..bd5f7f5985e 100644 --- a/core/math/plane.h +++ b/core/math/plane.h @@ -33,7 +33,7 @@ #include "core/math/vector3.h" -class Plane { +class _NO_DISCARD_CLASS_ Plane { public: Vector3 normal; real_t d; diff --git a/core/math/quat.h b/core/math/quat.h index a9a056b55d6..1e2ca5b7395 100644 --- a/core/math/quat.h +++ b/core/math/quat.h @@ -36,7 +36,7 @@ #include "core/math/vector3.h" #include "core/ustring.h" -class Quat { +class _NO_DISCARD_CLASS_ Quat { public: real_t x, y, z, w; @@ -127,7 +127,7 @@ public: w(p_q.w) { } - Quat operator=(const Quat &p_q) { + Quat &operator=(const Quat &p_q) { x = p_q.x; y = p_q.y; z = p_q.z; diff --git a/core/math/rect2.h b/core/math/rect2.h index e35d40cb21c..e9f6a70d061 100644 --- a/core/math/rect2.h +++ b/core/math/rect2.h @@ -35,7 +35,7 @@ struct Transform2D; -struct Rect2 { +struct _NO_DISCARD_CLASS_ Rect2 { Point2 position; Size2 size; @@ -259,7 +259,7 @@ struct Rect2 { } }; -struct Rect2i { +struct _NO_DISCARD_CLASS_ Rect2i { Point2i position; Size2i size; diff --git a/core/math/transform.h b/core/math/transform.h index 76ed518b92f..2a6b488cfbe 100644 --- a/core/math/transform.h +++ b/core/math/transform.h @@ -36,7 +36,7 @@ #include "core/math/plane.h" #include "core/pool_vector.h" -class Transform { +class _NO_DISCARD_CLASS_ Transform { public: Basis basis; Vector3 origin; diff --git a/core/math/transform_2d.h b/core/math/transform_2d.h index 3fad7e8d277..1ae61ce3889 100644 --- a/core/math/transform_2d.h +++ b/core/math/transform_2d.h @@ -34,7 +34,7 @@ #include "core/math/rect2.h" // also includes vector2, math_funcs, and ustring #include "core/pool_vector.h" -struct Transform2D { +struct _NO_DISCARD_CLASS_ Transform2D { // Warning #1: basis of Transform2D is stored differently from Basis. In terms of elements array, the basis matrix looks like "on paper": // M = (elements[0][0] elements[1][0]) // (elements[0][1] elements[1][1]) diff --git a/core/math/vector2.h b/core/math/vector2.h index c32abac0f1a..b2a8795d9d0 100644 --- a/core/math/vector2.h +++ b/core/math/vector2.h @@ -36,7 +36,7 @@ struct Vector2i; -struct Vector2 { +struct _NO_DISCARD_CLASS_ Vector2 { static const int AXIS_COUNT = 2; enum Axis { @@ -269,7 +269,7 @@ typedef Vector2 Point2; /* INTEGER STUFF */ -struct Vector2i { +struct _NO_DISCARD_CLASS_ Vector2i { enum Axis { AXIS_X, AXIS_Y, diff --git a/core/math/vector3.h b/core/math/vector3.h index dc71a3b0bf9..4a9d379bb12 100644 --- a/core/math/vector3.h +++ b/core/math/vector3.h @@ -36,7 +36,7 @@ class Basis; -struct Vector3 { +struct _NO_DISCARD_CLASS_ Vector3 { static const int AXIS_COUNT = 3; enum Axis { diff --git a/core/typedefs.h b/core/typedefs.h index f1335afe0b6..a21cf727908 100644 --- a/core/typedefs.h +++ b/core/typedefs.h @@ -69,6 +69,47 @@ #endif +// No discard allows the compiler to flag warnings if we don't use the return value of functions / classes +#ifndef _NO_DISCARD_ +// c++ 17 onwards +#if __cplusplus >= 201703L +#define _NO_DISCARD_ [[nodiscard]] +#else +// __warn_unused_result__ supported on clang and GCC +#if (defined(__clang__) || defined(__GNUC__)) && defined(__has_attribute) +#if __has_attribute(__warn_unused_result__) +#define _NO_DISCARD_ __attribute__((__warn_unused_result__)) +#endif +#endif + +// Visual Studio 2012 onwards +#if _MSC_VER >= 1700 +#define _NO_DISCARD_ _Check_return_ +#endif + +// If nothing supported, just noop the macro +#ifndef _NO_DISCARD_ +#define _NO_DISCARD_ +#endif +#endif // not c++ 17 +#endif // not defined _NO_DISCARD_ + +// In some cases _NO_DISCARD_ will get false positives, +// we can prevent the warning in specific cases by preceding the call with a cast. +#ifndef _ALLOW_DISCARD_ +#define _ALLOW_DISCARD_ (void) +#endif + +// GCC (prior to c++ 17) Does not seem to support no discard with classes, only functions. +// So we will use a specific macro for classes. +#ifndef _NO_DISCARD_CLASS_ +#if (defined(__clang__) || defined(_MSC_VER)) +#define _NO_DISCARD_CLASS_ _NO_DISCARD_ +#else +#define _NO_DISCARD_CLASS_ +#endif +#endif + //custom, gcc-safe offsetof, because gcc complains a lot. template T *_nullptr() { diff --git a/modules/bullet/cone_twist_joint_bullet.cpp b/modules/bullet/cone_twist_joint_bullet.cpp index b20579c6dd8..c14490e1632 100644 --- a/modules/bullet/cone_twist_joint_bullet.cpp +++ b/modules/bullet/cone_twist_joint_bullet.cpp @@ -43,14 +43,14 @@ ConeTwistJointBullet::ConeTwistJointBullet(RigidBodyBullet *rbA, RigidBodyBullet *rbB, const Transform &rbAFrame, const Transform &rbBFrame) : JointBullet() { Transform scaled_AFrame(rbAFrame.scaled(rbA->get_body_scale())); - scaled_AFrame.basis.rotref_posscale_decomposition(scaled_AFrame.basis); + _ALLOW_DISCARD_ scaled_AFrame.basis.rotref_posscale_decomposition(scaled_AFrame.basis); btTransform btFrameA; G_TO_B(scaled_AFrame, btFrameA); if (rbB) { Transform scaled_BFrame(rbBFrame.scaled(rbB->get_body_scale())); - scaled_BFrame.basis.rotref_posscale_decomposition(scaled_BFrame.basis); + _ALLOW_DISCARD_ scaled_BFrame.basis.rotref_posscale_decomposition(scaled_BFrame.basis); btTransform btFrameB; G_TO_B(scaled_BFrame, btFrameB); diff --git a/modules/bullet/generic_6dof_joint_bullet.cpp b/modules/bullet/generic_6dof_joint_bullet.cpp index 6046b2c4cea..08d420df5c6 100644 --- a/modules/bullet/generic_6dof_joint_bullet.cpp +++ b/modules/bullet/generic_6dof_joint_bullet.cpp @@ -44,7 +44,7 @@ Generic6DOFJointBullet::Generic6DOFJointBullet(RigidBodyBullet *rbA, RigidBodyBu JointBullet() { Transform scaled_AFrame(frameInA.scaled(rbA->get_body_scale())); - scaled_AFrame.basis.rotref_posscale_decomposition(scaled_AFrame.basis); + _ALLOW_DISCARD_ scaled_AFrame.basis.rotref_posscale_decomposition(scaled_AFrame.basis); btTransform btFrameA; G_TO_B(scaled_AFrame, btFrameA); @@ -52,7 +52,7 @@ Generic6DOFJointBullet::Generic6DOFJointBullet(RigidBodyBullet *rbA, RigidBodyBu if (rbB) { Transform scaled_BFrame(frameInB.scaled(rbB->get_body_scale())); - scaled_BFrame.basis.rotref_posscale_decomposition(scaled_BFrame.basis); + _ALLOW_DISCARD_ scaled_BFrame.basis.rotref_posscale_decomposition(scaled_BFrame.basis); btTransform btFrameB; G_TO_B(scaled_BFrame, btFrameB); diff --git a/modules/bullet/hinge_joint_bullet.cpp b/modules/bullet/hinge_joint_bullet.cpp index 4fa486b7afd..6eb55db782e 100644 --- a/modules/bullet/hinge_joint_bullet.cpp +++ b/modules/bullet/hinge_joint_bullet.cpp @@ -43,14 +43,14 @@ HingeJointBullet::HingeJointBullet(RigidBodyBullet *rbA, RigidBodyBullet *rbB, const Transform &frameA, const Transform &frameB) : JointBullet() { Transform scaled_AFrame(frameA.scaled(rbA->get_body_scale())); - scaled_AFrame.basis.rotref_posscale_decomposition(scaled_AFrame.basis); + _ALLOW_DISCARD_ scaled_AFrame.basis.rotref_posscale_decomposition(scaled_AFrame.basis); btTransform btFrameA; G_TO_B(scaled_AFrame, btFrameA); if (rbB) { Transform scaled_BFrame(frameB.scaled(rbB->get_body_scale())); - scaled_BFrame.basis.rotref_posscale_decomposition(scaled_BFrame.basis); + _ALLOW_DISCARD_ scaled_BFrame.basis.rotref_posscale_decomposition(scaled_BFrame.basis); btTransform btFrameB; G_TO_B(scaled_BFrame, btFrameB); diff --git a/modules/bullet/slider_joint_bullet.cpp b/modules/bullet/slider_joint_bullet.cpp index 303b957d47a..cb9408b36e9 100644 --- a/modules/bullet/slider_joint_bullet.cpp +++ b/modules/bullet/slider_joint_bullet.cpp @@ -43,14 +43,14 @@ SliderJointBullet::SliderJointBullet(RigidBodyBullet *rbA, RigidBodyBullet *rbB, const Transform &frameInA, const Transform &frameInB) : JointBullet() { Transform scaled_AFrame(frameInA.scaled(rbA->get_body_scale())); - scaled_AFrame.basis.rotref_posscale_decomposition(scaled_AFrame.basis); + _ALLOW_DISCARD_ scaled_AFrame.basis.rotref_posscale_decomposition(scaled_AFrame.basis); btTransform btFrameA; G_TO_B(scaled_AFrame, btFrameA); if (rbB) { Transform scaled_BFrame(frameInB.scaled(rbB->get_body_scale())); - scaled_BFrame.basis.rotref_posscale_decomposition(scaled_BFrame.basis); + _ALLOW_DISCARD_ scaled_BFrame.basis.rotref_posscale_decomposition(scaled_BFrame.basis); btTransform btFrameB; G_TO_B(scaled_BFrame, btFrameB); diff --git a/platform/osx/os_osx.mm b/platform/osx/os_osx.mm index b05370d5424..75ec81a9507 100644 --- a/platform/osx/os_osx.mm +++ b/platform/osx/os_osx.mm @@ -426,7 +426,7 @@ static NSCursor *cursorFromSelector(SEL selector, SEL fallback = nil) { - (void)windowDidBecomeKey:(NSNotification *)notification { if (OS_OSX::singleton->get_main_loop()) { - get_mouse_pos([OS_OSX::singleton->window_object mouseLocationOutsideOfEventStream]); + _ALLOW_DISCARD_ get_mouse_pos([OS_OSX::singleton->window_object mouseLocationOutsideOfEventStream]); OS_OSX::singleton->input->set_mouse_position(Point2(mouse_x, mouse_y)); OS_OSX::singleton->get_main_loop()->notification(MainLoop::NOTIFICATION_WM_FOCUS_IN); @@ -1370,7 +1370,7 @@ inline void sendPanEvent(double dx, double dy, int modifierFlags) { - (void)scrollWheel:(NSEvent *)event { double deltaX, deltaY; - get_mouse_pos([event locationInWindow]); + _ALLOW_DISCARD_ get_mouse_pos([event locationInWindow]); deltaX = [event scrollingDeltaX]; deltaY = [event scrollingDeltaY]; @@ -2167,7 +2167,7 @@ void OS_OSX::warp_mouse_position(const Point2 &p_to) { } void OS_OSX::update_real_mouse_position() { - get_mouse_pos([window_object mouseLocationOutsideOfEventStream]); + _ALLOW_DISCARD_ get_mouse_pos([window_object mouseLocationOutsideOfEventStream]); input->set_mouse_position(Point2(mouse_x, mouse_y)); } diff --git a/scene/2d/canvas_item.cpp b/scene/2d/canvas_item.cpp index d18f74d393e..f2413c8d466 100644 --- a/scene/2d/canvas_item.cpp +++ b/scene/2d/canvas_item.cpp @@ -1247,7 +1247,7 @@ void CanvasItem::set_notify_transform(bool p_enable) { if (notify_transform && is_inside_tree()) { //this ensures that invalid globals get resolved, so notifications can be received - get_global_transform(); + _ALLOW_DISCARD_ get_global_transform(); } }