From fe8799a53c15fbec1e4af76efbc35e31f5f6c7cf Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Mon, 21 Jan 2019 22:31:35 +0000 Subject: [PATCH 1/4] Introduce new math_defines.h header Running IWYU broke the compile on VS because of it's complicated handling of math.h constants. This provides a single header to access those constants, and we can use IWYU to force people to include it, so that devs on other platforms are less likely to write code that breaks on VS. Also, this should make it possible to run IWYU without breaking the VS build. --- src/explosion.cpp | 4 +--- src/math_defines.h | 17 +++++++++++++++++ src/rng.cpp | 6 ++---- tools/iwyu/cata.imp | 1 + tools/iwyu/vs.stdlib.imp | 3 +++ 5 files changed, 24 insertions(+), 7 deletions(-) create mode 100644 src/math_defines.h create mode 100644 tools/iwyu/vs.stdlib.imp diff --git a/src/explosion.cpp b/src/explosion.cpp index 491025abe9bde..ef7ae5dbb6782 100644 --- a/src/explosion.cpp +++ b/src/explosion.cpp @@ -12,6 +12,7 @@ #include "game.h" #include "item_factory.h" #include "json.h" +#include "math_defines.h" #include "map.h" #include "messages.h" #include "output.h" @@ -23,9 +24,6 @@ #include "vehicle.h" #include "vpart_position.h" -// For M_PI -#define _USE_MATH_DEFINES -#include #include #include diff --git a/src/math_defines.h b/src/math_defines.h new file mode 100644 index 0000000000000..c1a008754b82a --- /dev/null +++ b/src/math_defines.h @@ -0,0 +1,17 @@ +#ifndef CATA_MATH_DEFINES_H +#define CATA_MATH_DEFINES_H + +// On Visual Studio math.h only provides the defines (M_PI, etc.) if +// _USE_MATH_DEFINES is defined before including it. +// We centralize that requirement in this header so that IWYU can force this +// header to be included when such defines are used, via mappings. +// At time of writing only M_PI has a mapping, which is the most commonly used +// one. If more are needed, add them to tools/iwyu/vs.stdlib.imp. + +// Note that it's important to use math.h here, not cmath. See +// https://stackoverflow.com/questions/6563810/m-pi-works-with-math-h-but-not-with-cmath-in-visual-studio/6563891 + +#define _USE_MATH_DEFINES +#include + +#endif // CATA_MATH_DEFINES_H diff --git a/src/rng.cpp b/src/rng.cpp index ba59eb9183951..01ccdf8b32707 100644 --- a/src/rng.cpp +++ b/src/rng.cpp @@ -1,14 +1,12 @@ #include "rng.h" #include - -#include "output.h" - -#define _USE_MATH_DEFINES #include #include #include +#include "output.h" + long rng( long val1, long val2 ) { long minVal = ( val1 < val2 ) ? val1 : val2; diff --git a/tools/iwyu/cata.imp b/tools/iwyu/cata.imp index 97154170a7fc2..d278546cfff3d 100644 --- a/tools/iwyu/cata.imp +++ b/tools/iwyu/cata.imp @@ -1,4 +1,5 @@ [ { ref: "gcc.stdlib.imp" }, + { ref: "vs.stdlib.imp" }, { ref: "sdl.imp" }, ] diff --git a/tools/iwyu/vs.stdlib.imp b/tools/iwyu/vs.stdlib.imp new file mode 100644 index 0000000000000..6a78ced407f22 --- /dev/null +++ b/tools/iwyu/vs.stdlib.imp @@ -0,0 +1,3 @@ +[ + { symbol: ["M_PI", "private", "\"math_defines.h\"", "public"] }, +] From 81560ae3b56d03874480e74cc378c89e072811d7 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Mon, 21 Jan 2019 22:40:10 +0000 Subject: [PATCH 2/4] Add protected strings.h include This needs to be added by hand because it's non-Windows only, and IWYU will add it in a place where Windows will choke on it. --- src/sdltiles.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sdltiles.cpp b/src/sdltiles.cpp index 3e7ca38ad1d49..cd7d0a3c3579d 100644 --- a/src/sdltiles.cpp +++ b/src/sdltiles.cpp @@ -54,6 +54,8 @@ # ifndef strcasecmp # define strcasecmp StrCmpI # endif +#else +# include // for strcasecmp #endif #ifdef __ANDROID__ From 13aa8f9607269ca594cca890295a8f569bb41c1b Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Mon, 21 Jan 2019 22:41:47 +0000 Subject: [PATCH 3/4] Mapping for debug/deque Missed this before; highlighted by a VS compile error. --- tools/iwyu/gcc.stdlib.imp | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/iwyu/gcc.stdlib.imp b/tools/iwyu/gcc.stdlib.imp index e54a88f4ed2f3..483a1c8b51091 100644 --- a/tools/iwyu/gcc.stdlib.imp +++ b/tools/iwyu/gcc.stdlib.imp @@ -11,6 +11,7 @@ { include: ["", "private", "", "public"] }, { include: ["", "private", "", "public"] }, { include: ["", "private", "", "public"] }, + { include: ["", "private", "", "public"] }, { include: ["", "private", "", "public"] }, { include: ["", "private", "", "public"] }, { include: ["", "private", "", "public"] }, From b9662f5bb4913ae229598a1bc8503354a5f7927a Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Mon, 21 Jan 2019 22:49:16 +0000 Subject: [PATCH 4/4] Map nanosleep and struct timespec to posix_time.h IWYU needs to know that this is our compatibility header in order to use the correct includes. --- src/posix_time.h | 8 +++++--- tools/iwyu/vs.stdlib.imp | 2 ++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/posix_time.h b/src/posix_time.h index e29d86e75dc62..3df188333cabc 100644 --- a/src/posix_time.h +++ b/src/posix_time.h @@ -2,6 +2,11 @@ #ifndef TIME_SPEC_H #define TIME_SPEC_H +// Compatibility header. On POSIX, just include . On Windows, provide +// our own nanosleep implementation. + +#include // IWYU pragma: keep + #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ /* Windows platforms. */ @@ -9,9 +14,6 @@ together from GNUlib (http://www.gnu.org/software/gnulib/), which is licensed under the GPLv3. */ -#include -#include - enum { BILLION = 1000 * 1000 * 1000 }; # ifdef __cplusplus diff --git a/tools/iwyu/vs.stdlib.imp b/tools/iwyu/vs.stdlib.imp index 6a78ced407f22..07db16d9b8d80 100644 --- a/tools/iwyu/vs.stdlib.imp +++ b/tools/iwyu/vs.stdlib.imp @@ -1,3 +1,5 @@ [ { symbol: ["M_PI", "private", "\"math_defines.h\"", "public"] }, + { symbol: ["nanosleep", "private", "\"posix_time.h\"", "public"] }, + { symbol: ["timespec", "private", "\"posix_time.h\"", "public"] }, ]