Repository: HandmadeMath/HandmadeMath (c)

Issue:
Introduce "safe" float comparison 
Introduce a safe form of comparing floating point. == and != should not be used when comparing floating numbers. 

`./HandmadeMath.h:854:31: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
    hmm_bool Result = (Left.X == Right.X && Left.Y == Right.Y);
                       ~~~~~~ ^  ~~~~~~~
./HandmadeMath.h:854:52: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
    hmm_bool Result = (Left.X == Right.X && Left.Y == Right.Y);
                                            ~~~~~~ ^  ~~~~~~~
./HandmadeMath.h:861:31: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
    hmm_bool Result = (Left.X == Right.X && Left.Y == Right.Y && Left.Z == Right.Z);
                       ~~~~~~ ^  ~~~~~~~
./HandmadeMath.h:861:52: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
    hmm_bool Result = (Left.X == Right.X && Left.Y == Right.Y && Left.Z == Right.Z);
                                            ~~~~~~ ^  ~~~~~~~
./HandmadeMath.h:861:73: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
    hmm_bool Result = (Left.X == Right.X && Left.Y == Right.Y && Left.Z == Right.Z);
                                                                 ~~~~~~ ^  ~~~~~~~
./HandmadeMath.h:868:31: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
    hmm_bool Result = (Left.X == Right.X && Left.Y == Right.Y && Left.Z == Right.Z && Left.W == Right.W);
                       ~~~~~~ ^  ~~~~~~~
./HandmadeMath.h:868:52: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
    hmm_bool Result = (Left.X == Right.X && Left.Y == Right.Y && Left.Z == Right.Z && Left.W == Right.W);
                                            ~~~~~~ ^  ~~~~~~~
./HandmadeMath.h:868:73: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
    hmm_bool Result = (Left.X == Right.X && Left.Y == Right.Y && Left.Z == Right.Z && Left.W == Right.W);
                                                                 ~~~~~~ ^  ~~~~~~~
./HandmadeMath.h:868:94: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]
    hmm_bool Result = (Left.X == Right.X && Left.Y == Right.Y && Left.Z == Right.Z && Left.W == Right.W);
`


Discussion:
Which functions are doing this?
Actually these are just the Equals functions. So this warning really isnt valid, might wanna disable this warning on clang
That sounds like the better option. I'd imagine this is rarely used anyway, and if it is, it's probably comparing to zero or something.
You’re both right, comparing to known values (like exactly identity matrix) is about the only use case here. Anything beyond that and users will prefer to write their own custom little comparison inlined in their code.

I would actually recommend simply deleting the comparison function you have instead of silencing the warnings. It’s just a little extra bloat and not actually a useful function. But silencing isn’t a bad option either. It just means you’ll have more maintenance in terms of silencing warnings for updating or different compilers... So I would prefer to just delete it and not maintain the function at all.
Do we have a matrix equals function?? We should definitely remove that. I can see a use for the vector case, but really only in the situation where you’re checking for zero. In that case I suppose checking each element is probably faster than getting the length (but I have not done any tests to verify that!)
I know we’re not really living in crazy package land here, but in the interest of backwards compatibility, should we silence the warnings for now and remove the function(s) in a future major release? (Like maybe alongside handedness stuff.)
Oh my mistake, I thought I was looking at matrix comparisons. I'm not actually sure how often people use vector comparisons. I never have myself 🤷‍♂ 

To check for all zeros in a vector I do `dot(a, b) == 0`, and write this inline. I've never had a case where I actually used a vector equal operator. In my own math header I use `_mm_cmpeq_ps` and `_mm_cmpneq_ps` simply because I happened to see these functions. I don't have any scalar overloads, so no warnings to silence. I'm actually probably going to just delete the == and != operators.

For deprecation there are a few options I can think of.

* Just delete function and make a note in the release notes on the breaking change.
* Use a macro to mark functions as deprecated, and print a note upon compilation. Then eventually delete the function, or move it to a separate deprecated section.
* The deprecated section could be an ifdef, or even just another header. For example, HandmadeMathDeprecated.h to be included after HandmadeMath.h is included.
I’m all of option 2 this is what Unreal Engine 4 does. They say “Hey X function will be deleted in the next version” with a #warning and then remove it next version. Gives people time to update their usage of the library 
@bvisness Any opinions on what we should do here. Looking to close this ticket this weekend  
I think we should definitely silence the float comparison warning. The kind of bug it’s trying to prevent is just not applicable here. And I think we can go ahead and throw in a deprecation warning of our own, and plan to remove the operators/functions in version 2.0. 
Cool. I’ll push a fix sometimes today silencing this warning 
I'm torn on whether or not to delete the vector equals functions for 2.0. Unsurprisingly, we use them everywhere in the tests, so we'd have to reimplement them there. But the main reason I'm hesitant to delete them is that they're totally unambiguous, and not really any work to maintain.

At the moment I'd prefer to just silence the warnings and keep the functions around, even for 2.0. Feel free to change my mind!

Patch:
diff --git a/HandmadeMath.h b/HandmadeMath.h
--- a/HandmadeMath.h
+++ b/HandmadeMath.h
@@ -130,14 +130,23 @@
 #pragma warning(disable:4201)
 #endif
 
-#ifdef __clang__
+#if defined(__GNUC__) || defined(__clang__)
 #pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wfloat-equal"
+#if defined(__GNUC__) && (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
+#pragma GCC diagnostic ignored "-Wmissing-braces"
+#endif
+#ifdef __clang__
 #pragma GCC diagnostic ignored "-Wgnu-anonymous-struct"
 #endif
+#endif
 
-#if defined(__GNUC__) && (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wmissing-braces"
+#if defined(__GNUC__) || defined(__clang__)
+#define HMM_DEPRECATED(msg) __attribute__((deprecated(msg)))
+#elif defined(_MSC_VER)
+#define HMM_DEPRECATED(msg) __declspec(deprecated(msg))
+#else
+#define HMM_DEPRECATED(msg)
 #endif
 
 #ifdef __cplusplus
@@ -353,8 +362,7 @@ typedef union hmm_mat4
 #ifdef HANDMADE_MATH__USE_SSE
     __m128 Columns[4];
 
-    // DEPRECATED. Our matrices are column-major, so this was named
-    // incorrectly. Use Columns instead.
+    HMM_DEPRECATED("Our matrices are column-major, so this was named incorrectly. Use Columns instead.")
     __m128 Rows[4];
 #endif
 
@@ -2769,11 +2777,7 @@ HMM_INLINE hmm_bool operator!=(hmm_vec4 Left, hmm_vec4 Right)
 
 #endif /* __cplusplus */
 
-#ifdef __clang__
-#pragma GCC diagnostic pop
-#endif
-
-#if defined(__GNUC__) && (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
+#if defined(__GNUC__) || defined(__clang__)
 #pragma GCC diagnostic pop
 #endif
 


Test patch:
diff --git a/test/Makefile b/test/Makefile
--- a/test/Makefile
+++ b/test/Makefile
@@ -1,6 +1,6 @@
 BUILD_DIR=./build
 
-CXXFLAGS+=-g -Wall -Wextra -pthread -Wno-missing-braces -Wno-missing-field-initializers
+CXXFLAGS+=-g -Wall -Wextra -pthread -Wno-missing-braces -Wno-missing-field-initializers -Wfloat-equal
 
 all: c c_no_sse cpp cpp_no_sse build_c_without_coverage build_cpp_without_coverage
 
diff --git a/test/categories/ScalarMath.h b/test/categories/ScalarMath.h
--- a/test/categories/ScalarMath.h
+++ b/test/categories/ScalarMath.h
@@ -74,9 +74,9 @@ TEST(ScalarMath, Power)
 
 TEST(ScalarMath, PowerF)
 {
-    EXPECT_FLOAT_EQ(HMM_PowerF(2.0f, 0), 1.0f);
-    EXPECT_NEAR(HMM_PowerF(2.0f, 4.1), 17.148376f, 0.0001f);
-    EXPECT_NEAR(HMM_PowerF(2.0f, -2.5), 0.176777f, 0.0001f);
+    EXPECT_FLOAT_EQ(HMM_PowerF(2.0f, 0.0f), 1.0f);
+    EXPECT_NEAR(HMM_PowerF(2.0f, 4.1f), 17.148376f, 0.0001f);
+    EXPECT_NEAR(HMM_PowerF(2.0f, -2.5f), 0.176777f, 0.0001f);
 }
 
 TEST(ScalarMath, Lerp)