Improve handling of platform-specific build options (#2162)

* Opt into POSIX functions with `_POSIX_C_SOURCE` before any system
  headers are `#include`d
* Allow user to override `make develop`'s `WARNFLAGS` and `CXXFLAGS`
* Use `make develop` on 32-bit Cygwin with sanitizers disabled
* Do not unnecessarily redefine `fseek` and `ftell`
* Disable a false-positive `-Wno-null-dereference` on 32-bit Cygwin

Co-authored-by: ISSOtm <[email protected]>

.clang-format

@@ -42,6 +42,8 @@ EmptyLineBeforeAccessModifier: Leave
 FixNamespaceComments: false
 IncludeBlocks: Regroup
 IncludeCategories:
+  - Regex: '^"platform\.hpp"$'
+    Priority: -1
   - Regex: '^<sys/'
     Priority: 0
   - Regex: '^<'

.github/workflows/testing.yml

@@ -371,8 +371,12 @@ jobs:
             python312-imaging
       - name: Build using Make
         if: matrix.arch == 'x86'
-        run: | # Cygwin doesn't support `make develop` sanitizers ASan or UBSan, so we don't use it.
+        run: | # Cygwin doesn't support `make develop` sanitizers ASan or UBSan, so we disable them.
-          make -k -j "$(getconf _NPROCESSORS_ONLN)" Q=
+          : # 32-bit Cygwin stopped updating since 2022-11-23, and its g++ 11.3.0 gives a false
+          : # positive "potential null pointer dereference" error on `patch.pcOffset -= pcShift;`
+          : # in src/asm/output.cpp, so we disable that warning option.
+          make develop -k -j "$(getconf _NPROCESSORS_ONLN)" Q= \
+               WARNFLAGS="-fno-sanitize=all -Wno-null-dereference"
       - name: Install using Make
         if: matrix.arch == 'x86'
         run: |

Makefile

@@ -222,7 +222,7 @@ install: all
 # continous integration infrastructure return failure.
 # The rationale for some of the flags is documented in the CMakeLists.
 develop:
-	$Q${MAKE} WARNFLAGS="${WARNFLAGS} -Werror -Wextra \
+	$Q${MAKE} WARNFLAGS="-Werror -Wextra \
 		-Walloc-zero -Wcast-align -Wcast-qual -Wduplicated-branches -Wduplicated-cond \
 		-Wfloat-equal -Wlogical-op -Wnull-dereference -Wold-style-cast -Wshift-overflow=2 \
 		-Wstringop-overflow=4 -Wtrampolines -Wundef -Wuninitialized -Wunused -Wshadow \
@@ -232,8 +232,9 @@ develop:
 		-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_GLIBCXX_SANITIZE_VECTOR \
 		-D_GLIBCXX_VERBOSE_ASSERT -D_GLIBCXX_EXTERN_TEMPLATE=0 \
 		-D_LIBCPP_DEBUG -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG \
-		-fsanitize=address -fsanitize=undefined -fsanitize=float-divide-by-zero" \
+		-fsanitize=address -fsanitize=undefined -fsanitize=float-divide-by-zero \
-		CXXFLAGS="-ggdb3 -Og -fno-omit-frame-pointer -fno-optimize-sibling-calls"
+		${WARNFLAGS}" \
+		CXXFLAGS="-ggdb3 -Og -fno-omit-frame-pointer -fno-optimize-sibling-calls ${CXXFLAGS}"
 
 # Target used in development to debug with gdb.
 debug:

include/file.hpp

@@ -3,6 +3,8 @@
 #ifndef RGBDS_FILE_HPP
 #define RGBDS_FILE_HPP
 
+#include "platform.hpp"
+
 #include <fcntl.h>
 #include <fstream>
 #include <ios>
@@ -12,7 +14,6 @@
 #include <variant>
 
 #include "helpers.hpp" // assume
-#include "platform.hpp"
 
 class File {
 	std::variant<std::streambuf *, std::filebuf> _file;

include/platform.hpp

@@ -3,6 +3,12 @@
 #ifndef RGBDS_PLATFORM_HPP
 #define RGBDS_PLATFORM_HPP
 
+// MingGW and Cygwin may need POSIX functions which are not standard C explicitly enabled
+// Make sure to keep this before any system header!
+#if (defined(__MINGW32__) || defined(__CYGWIN__)) && !defined(_POSIX_C_SOURCE)
+	#define _POSIX_C_SOURCE 200809L
+#endif
+
 // MSVC doesn't have str(n)casecmp, use a suitable replacement
 #ifdef _MSC_VER
 	#include <string.h> // IWYU pragma: export
@@ -55,17 +61,6 @@
 	#define setmode(fd, mode) (0)
 #endif
 
-// Windows has 32-bit `long`, which limits `fseek` and `ftell` to 2 GiB
-#if defined(_MSC_VER) || defined(__MINGW32__)
-	#define fseek _fseeki64
-	#define ftell _ftelli64
-#endif
-
-// MingGW and Cygwin may need POSIX functions which are not standard C explicitly enabled
-#if (defined(__MINGW32__) || defined(__CYGWIN__)) && !defined(_POSIX_C_SOURCE)
-	#define _POSIX_C_SOURCE 200809L
-#endif
-
 // Apple has deprecated `sprintf` since Xcode 14 (for macOS 13), but we use it solely in
 // contexts where both the size of the buffer *and* max size of the printed string are
 // known statically, which GCC thus checks for.

src/asm/fstack.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp" // strncasecmp
+
 #include "asm/fstack.hpp"
 #include <sys/stat.h>
 
@@ -22,7 +24,6 @@
 #include "helpers.hpp"
 #include "itertools.hpp" // reversed
 #include "linkdefs.hpp"
-#include "platform.hpp" // strncasecmp
 #include "verbosity.hpp"
 
 #include "asm/intern.hpp"

src/asm/lexer.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "asm/lexer.hpp"
 #include <sys/stat.h>
 
@@ -27,7 +29,6 @@
 #include <vector>
 
 #include "helpers.hpp"
-#include "platform.hpp"
 #include "style.hpp"
 #include "util.hpp"
 #include "verbosity.hpp"

src/asm/main.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "asm/main.hpp"
 
 #include <algorithm>
@@ -23,8 +25,7 @@
 #include "diagnostics.hpp"
 #include "helpers.hpp"
 #include "parser.hpp" // Generated from parser.y
-#include "platform.hpp"
+#include "style.hpp"  // style_Parse
-#include "style.hpp" // style_Parse
 #include "usage.hpp"
 #include "util.hpp" // UpperMap
 #include "verbosity.hpp"

src/asm/output.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "asm/output.hpp"
 
 #include <algorithm>
@@ -16,7 +18,6 @@
 
 #include "helpers.hpp" // assume, Defer
 #include "linkdefs.hpp"
-#include "platform.hpp"
 #include "util.hpp" // xfclose
 
 #include "asm/charmap.hpp"

src/asm/section.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp" // fseek
+
 #include "asm/section.hpp"
 
 #include <algorithm>
@@ -21,8 +23,7 @@
 #include "helpers.hpp"
 #include "itertools.hpp" // InsertionOrderedMap
 #include "linkdefs.hpp"
-#include "platform.hpp" // fseek
+#include "util.hpp" // xfclose, seekSize
-#include "util.hpp"     // xfclose, seekSize
 
 #include "asm/fstack.hpp"
 #include "asm/lexer.hpp"

src/backtrace.cpp

@@ -1,12 +1,13 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp" // strcasecmp
+
 #include "backtrace.hpp"
 
 #include <optional>
 #include <stdint.h>
 
-#include "platform.hpp" // strcasecmp
+#include "util.hpp" // parseWholeNumber
-#include "util.hpp"     // parseWholeNumber
 
 Tracing tracing;
 

src/fix/fix.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "fix/fix.hpp"
 #include <sys/stat.h>
 
@@ -14,7 +16,6 @@
 
 #include "diagnostics.hpp"
 #include "helpers.hpp"
-#include "platform.hpp"
 #include "util.hpp" // xclose, xfclose
 
 #include "fix/main.hpp"

src/fix/main.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "fix/main.hpp"
 
 #include <errno.h>
@@ -15,7 +17,6 @@
 #include "cli.hpp"
 #include "diagnostics.hpp"
 #include "helpers.hpp"
-#include "platform.hpp"
 #include "style.hpp"
 #include "usage.hpp"
 #include "util.hpp"

src/fix/mbc.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp" // strcasecmp
+
 #include "fix/mbc.hpp"
 
 #include <optional>
@@ -10,8 +12,7 @@
 #include <unordered_map>
 #include <utility>
 
-#include "helpers.hpp"  // unreachable_
+#include "helpers.hpp" // unreachable_
-#include "platform.hpp" // strcasecmp
 #include "util.hpp"
 
 #include "fix/warning.hpp"

src/gfx/main.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "gfx/main.hpp"
 
 #include <inttypes.h>
@@ -19,7 +21,6 @@
 #include "diagnostics.hpp"
 #include "file.hpp"
 #include "helpers.hpp"
-#include "platform.hpp"
 #include "style.hpp" // style_Parse
 #include "usage.hpp"
 #include "util.hpp"

src/gfx/pal_spec.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "gfx/pal_spec.hpp"
 
 #include <algorithm>
@@ -22,7 +24,6 @@
 #include "diagnostics.hpp"
 #include "file.hpp"
 #include "helpers.hpp"
-#include "platform.hpp"
 #include "util.hpp" // UpperMap, parseDigit
 
 #include "gfx/main.hpp"

src/link/assign.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "link/assign.hpp"
 
 #include <algorithm>
@@ -16,7 +18,6 @@
 #include "helpers.hpp"
 #include "itertools.hpp"
 #include "linkdefs.hpp"
-#include "platform.hpp"
 #include "verbosity.hpp"
 
 #include "link/main.hpp"

src/link/object.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "link/object.hpp"
 
 #include <deque>
@@ -18,7 +20,6 @@
 
 #include "helpers.hpp"
 #include "linkdefs.hpp"
-#include "platform.hpp"
 #include "util.hpp" // xfclose
 #include "verbosity.hpp"
 #include "version.hpp"

src/link/output.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "link/output.hpp"
 
 #include <algorithm>
@@ -19,7 +21,6 @@
 #include "extern/utf8decoder.hpp"
 #include "helpers.hpp"
 #include "linkdefs.hpp"
-#include "platform.hpp"
 #include "util.hpp"
 
 #include "link/main.hpp"

src/link/sdas_obj.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "link/sdas_obj.hpp"
 
 #include <inttypes.h>
@@ -14,7 +16,6 @@
 
 #include "helpers.hpp" // assume, literal_strlen
 #include "linkdefs.hpp"
-#include "platform.hpp"
 #include "util.hpp" // parseWholeNumber
 
 #include "link/fstack.hpp"

src/style.cpp

@@ -3,14 +3,14 @@
 // This implementation was based on https://github.com/agauniyal/rang/
 // and adapted for RGBDS.
 
+#include "platform.hpp" // isatty, strcasecmp
+
 #include "style.hpp"
 
 #include <stdio.h>
 #include <stdlib.h> // getenv
 #include <string.h>
 
-#include "platform.hpp" // isatty, strcasecmp
-
 #if !STYLE_ANSI
 // clang-format off: maintain `include` order
 	#define WIN32_LEAN_AND_MEAN // Include less from `windows.h`

src/usage.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "usage.hpp"
 
 #include <stdarg.h>
@@ -8,7 +10,6 @@
 
 #include "diagnostics.hpp" // vfatalx
 #include "helpers.hpp"
-#include "platform.hpp"
 #include "style.hpp"
 #include "util.hpp" // parseWholeNumber
 #include "version.hpp"

src/util.cpp

@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: MIT
 
+#include "platform.hpp"
+
 #include "util.hpp"
 
 #include <errno.h>
@@ -9,7 +11,6 @@
 #include <string.h> // strspn
 
 #include "helpers.hpp" // assume
-#include "platform.hpp"
 
 int xfclose(FILE *file) {
 	if (file == stdin || file == stdout || file == stderr) {