lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lmi] PATCH: MSVC compilation fix


From: Vadim Zeitlin
Subject: [lmi] PATCH: MSVC compilation fix
Date: Thu, 2 Jun 2022 17:11:06 +0200

 Hello,

 I'd like to submit a small patch, which is basically this:

---------------------------------- >8 --------------------------------------
diff --git a/fdlibm.hpp b/fdlibm.hpp
index 28cac3135..4c99b1daf 100644
--- a/fdlibm.hpp
+++ b/fdlibm.hpp
@@ -51,6 +51,14 @@
 #   endif // !defined __FLOAT_WORD_ORDER__ && defined __BYTE_ORDER__
 #endif // defined __clang__

+// And MSVC maintainers don't believe in having different endianness
+// values at all, so the compiler never predefines these symbols.
+#if defined LMI_MSC
+    #define __ORDER_BIG_ENDIAN__ 4321
+    #define __ORDER_LITTLE_ENDIAN__ 1234
+    #define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__
+#endif // defined LMI_MSC
+
 #if !defined __FLOAT_WORD_ORDER__ || \
     !defined __ORDER_BIG_ENDIAN__ || \
     !defined __ORDER_LITTLE_ENDIAN__
---------------------------------- >8 --------------------------------------

to fix MSVC compilation. I can keep it in my own branch, of course, but as
long as we support MSVC in other places, it doesn't seem to be a huge
stretch to account for it here too.

 If you agree, this patch, as well as a cosmetic change to replace
__clang__ with LMI_CLANG for consistency in the check just above, is 
available in xanadu/msvc-endianness-macros branch, corresponding to
the PR 212 (https://github.com/let-me-illustrate/lmi/pull/212).


 Finally, to finish with MSVC-related topics, there are a couple of new
warnings in the recently added code:

fdlibm_log1p.c(192): warning C4701: potentially uninitialized local variable 
'f' used
fdlibm_log1p.c(193): warning C4701: potentially uninitialized local variable 
'hu' used
fdlibm_log1p.c(196): warning C4701: potentially uninitialized local variable 
'c' used
fdlibm_expm1.c(234): warning C4701: potentially uninitialized local variable 
'c' used

I don't think any of them indicates any actual problems, but I wonder if it
might still make sense to initialize these variables just to avoid even
having to think about it. E.g. in fdlibm_expm1.c I can see that "c" must be
initialized if the control flow reaches the line 234, because otherwise "k"
would have been set to 0 above and we would have returned before using it,
but the comment saying "c is 0" on that line is still wrong, as it's really
uninitialized there. And while, again, it doesn't really matter, it seems
that using "double c=0" wouldn't do any harm and would avoid the issue
entirely. Please let me know if you'd like me to make a patch doing this by
chance. Otherwise I'll just disable this warning at MSVC project level
(there are, BTW, still plenty of other warnings in MSVC builds related to
using enums and doubles in arithmetic expressions, that I still think ought
to be really fixed rather than suppressed, but I had already posted about
them, so I won't be repeating this once again).

 Thanks in advance,
VZ

Attachment: pgpJm1OY_J6ZR.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]