Dev/win compile (#402)

Collection of fixes to make aare build on Windows. Requires the previous
PRs for compilation to succeed.

- Replaced `or` keyword with ||
- updated cmake file
- fix for 32 bit long in fseek

closes #395 
closes #396

CMakeLists.txt

@@ -254,18 +254,30 @@ target_compile_features(aare_compiler_flags INTERFACE cxx_std_17)
 # MSVC specific #
 # ##############################################################################
 if(MSVC)
-  add_compile_definitions(AARE_MSVC)
-  if(CMAKE_BUILD_TYPE STREQUAL "Release")
-    message(STATUS "Release build")
-    target_compile_options(aare_compiler_flags INTERFACE /O2)
-  else()
-    message(STATUS "Debug build")
-    target_compile_options(aare_compiler_flags
-                           INTERFACE /Od /Zi /MDd /D_ITERATOR_DEBUG_LEVEL=2)
-    target_link_options(aare_compiler_flags INTERFACE /DEBUG:FULL)
-  endif()
-  target_compile_options(aare_compiler_flags INTERFACE /w # disable warnings
+  # CMake sets the per-configuration optimisation, debug-info and
+  # runtime-library flags itself, so only warnings and conformance here.
+  target_compile_options(
+    aare_compiler_flags
+    INTERFACE /W4 # -Wall -Wextra
+              /permissive- # standard conformance, like CMAKE_CXX_EXTENSIONS OFF
+              /utf-8 # sources are UTF-8
+              /we4715 # not all control paths return a value, like
+                      # -Werror=return-type
+              /wd4244 # narrowing conversions: -Wconversion is not enabled
+              /wd4267 # on gcc/clang either
+              /wd4068 # unknown pragma: the GCC diagnostic pragmas
+              /wd4324 # structure padded due to alignas: intended
   )
+  target_compile_definitions(aare_compiler_flags
+                             INTERFACE _CRT_SECURE_NO_WARNINGS)
+
+  if(AARE_WARNINGS_AS_ERRORS)
+    target_compile_options(aare_compiler_flags INTERFACE /WX)
+  endif()
+
+  if(AARE_ASAN)
+    message(WARNING "AARE_ASAN is not supported with MSVC. Ignoring.")
+  endif()
 
 else()
   # ############################################################################
@@ -318,22 +330,22 @@ else()
     endif()
   endif()
 
+  if(AARE_ASAN)
+    message(STATUS "AddressSanitizer enabled")
+    target_compile_options(
+      aare_compiler_flags INTERFACE -fsanitize=address,undefined,pointer-compare
+                                    -fno-omit-frame-pointer)
+    target_link_libraries(
+      aare_compiler_flags INTERFACE -fsanitize=address,undefined,pointer-compare
+                                    -fno-omit-frame-pointer)
+  endif()
+
 endif() # GCC/Clang specific
 
 if(AARE_PYTHON_BINDINGS)
   add_subdirectory(python)
 endif()
 
-if(AARE_ASAN)
-  message(STATUS "AddressSanitizer enabled")
-  target_compile_options(
-    aare_compiler_flags INTERFACE -fsanitize=address,undefined,pointer-compare
-                                  -fno-omit-frame-pointer)
-  target_link_libraries(
-    aare_compiler_flags INTERFACE -fsanitize=address,undefined,pointer-compare
-                                  -fno-omit-frame-pointer)
-endif()
-
 if(AARE_TESTS)
   enable_testing()
   add_subdirectory(tests)

include/aare/FilePtr.hpp

@@ -1,11 +1,29 @@
 // SPDX-License-Identifier: MPL-2.0
 #pragma once
 #include "aare/defs.hpp"
+#include <cstdint>
 #include <cstdio>
 #include <filesystem>
 
 namespace aare {
 
+// MSVC's long is 32 bits, so fseek/ftell stop working past 2 GB there
+inline int fseek64(FILE *fp, int64_t offset, int whence) {
+#ifdef _WIN32
+    return _fseeki64(fp, offset, whence);
+#else
+    return fseek(fp, static_cast<long>(offset), whence);
+#endif
+}
+
+inline int64_t ftell64(FILE *fp) {
+#ifdef _WIN32
+    return _ftelli64(fp);
+#else
+    return ftell(fp);
+#endif
+}
+
 /**
  * \brief RAII wrapper for FILE pointer
  */
@@ -23,7 +41,7 @@ class FilePtr {
     FILE *get();
     ssize_t tell();
     void seek(ssize_t offset, int whence = SEEK_SET) {
-        if (fseek(fp_, offset, whence) != 0)
+        if (fseek64(fp_, offset, whence) != 0)
             throw std::runtime_error("Error seeking in file");
     }
     std::string error_msg();

include/aare/NumpyFile.hpp

@@ -2,6 +2,7 @@
 #pragma once
 #include "aare/Dtype.hpp"
 #include "aare/FileInterface.hpp"
+#include "aare/FilePtr.hpp"
 #include "aare/NumpyHelpers.hpp"
 #include "aare/defs.hpp"
 
@@ -77,7 +78,7 @@ class NumpyFile : public FileInterface {
      */
     template <typename T, size_t NDim> NDArray<T, NDim> load() {
         NDArray<T, NDim> arr(make_shape<NDim>(m_header.shape));
-        if (fseek(fp, static_cast<long>(header_size), SEEK_SET)) {
+        if (fseek64(fp, header_size, SEEK_SET)) {
             throw std::runtime_error(LOCATION +
                                      "Error seeking to the start of the data");
         }

python/src/bind_ClusterVector.hpp

@@ -124,7 +124,7 @@ void define_ClusterVector(py::module &m, const std::string &typestr) {
         .def_property_readonly(
             "fmt",
             [typestr](ClusterVector<ClusterType> &self) {
-                return fmt_format<ClusterType>;
+                return fmt_format<ClusterType>();
             },
             "PEP 3118 format string for one stored cluster.")
 
@@ -145,11 +145,11 @@ void define_ClusterVector(py::module &m, const std::string &typestr) {
         .def_buffer(
             [typestr](ClusterVector<ClusterType> &self) -> py::buffer_info {
                 return py::buffer_info(
-                    self.data(),             /* Pointer to buffer */
-                    self.item_size(),        /* Size of one scalar */
-                    fmt_format<ClusterType>, /* Format descriptor */
-                    1,                       /* Number of dimensions */
-                    {self.size()},           /* Buffer dimensions */
+                    self.data(),               /* Pointer to buffer */
+                    self.item_size(),          /* Size of one scalar */
+                    fmt_format<ClusterType>(), /* Format descriptor */
+                    1,                         /* Number of dimensions */
+                    {self.size()},             /* Buffer dimensions */
                     {self.item_size()} /* Strides (in bytes) for each index */
                 );
             });

python/src/np_helper.hpp

@@ -112,8 +112,13 @@ struct fmt_format_trait<Cluster<T, ClusterSizeX, ClusterSizeY, CoordType>> {
     }
 };
 
-template <typename ClusterType>
-auto fmt_format = fmt_format_trait<ClusterType>::value();
+// Function-local static rather than a variable template: MSVC does not run
+// the dynamic initializer of a variable template instantiated from inside
+// another template, which left the format string empty.
+template <typename ClusterType> const std::string &fmt_format() {
+    static const std::string value = fmt_format_trait<ClusterType>::value();
+    return value;
+}
 
 /**
  * Helper function to allocate image data given item size and shape

src/ClusterVector.test.cpp

@@ -181,7 +181,8 @@ TEST_CASE("Summing 3x1 clusters of int64") {
 
     Cluster<int32_t, 3, 1> c3 = {11, 12, {13, 14, 15}};
     cv.push_back(c3);
-    REQUIRE(cv.capacity() == 4);
+    // growth factor is implementation defined (2x libstdc++, 1.5x MSVC)
+    REQUIRE(cv.capacity() >= 3);
     REQUIRE(cv.size() == 3);
 
     /*
@@ -237,7 +238,8 @@ TEST_CASE("Push back more than initial capacity") {
     Cluster<int32_t, 2, 2> c3 = {11, 12, {13, 14, 15, 16}};
     cv.push_back(c3);
     REQUIRE(cv.size() == 3);
-    REQUIRE(cv.capacity() == 4);
+    // growth factor is implementation defined (2x libstdc++, 1.5x MSVC)
+    REQUIRE(cv.capacity() >= 3);
 
     Cluster<int32_t, 2, 2> *ptr =
         reinterpret_cast<Cluster<int32_t, 2, 2> *>(cv.data());

src/FilePtr.cpp

@@ -27,7 +27,7 @@ FilePtr::operator bool() const noexcept { return fp_ != nullptr; }
 FILE *FilePtr::get() { return fp_; }
 
 ssize_t FilePtr::tell() {
-    auto pos = ftell(fp_);
+    auto pos = ftell64(fp_);
     if (pos == -1)
         throw std::runtime_error(
             fmt::format("Error getting file position: {}", error_msg()));

src/NumpyFile.cpp

@@ -45,7 +45,7 @@ void NumpyFile::write_impl(void *data, uint64_t size) {
     if (!(m_mode == "w" || m_mode == "a")) {
         throw std::invalid_argument("File not open for writing");
     }
-    if (fseek(fp, 0, SEEK_END))
+    if (fseek64(fp, 0, SEEK_END))
         throw std::runtime_error("Could not seek to end of file");
     size_t const rc = fwrite(data, size, 1, fp);
     if (rc != 1) {
@@ -67,8 +67,8 @@ void NumpyFile::get_frame_into(size_t frame_number, std::byte *image_buf) {
     if (frame_number > m_header.shape[0]) {
         throw std::invalid_argument("Frame number out of range");
     }
-    if (fseek(fp, header_size + frame_number * m_bytes_per_frame,
-              SEEK_SET)) // NOLINT
+    if (fseek64(fp, header_size + frame_number * m_bytes_per_frame,
+                SEEK_SET)) // NOLINT
         throw std::runtime_error("Could not seek to frame");
 
     size_t const rc = fread(image_buf, m_bytes_per_frame, 1, fp);
@@ -100,15 +100,15 @@ void NumpyFile::read_into(std::byte *image_buf, size_t n_frames) {
 NumpyFile::~NumpyFile() noexcept {
     if (m_mode == "w" || m_mode == "a") {
         // determine number of frames
-        if (fseek(fp, 0, SEEK_END)) {
+        if (fseek64(fp, 0, SEEK_END)) {
             std::cout << "Could not seek to end of file" << std::endl;
         }
-        size_t const file_size = ftell(fp);
+        size_t const file_size = ftell64(fp);
         size_t const data_size = file_size - initial_header_len;
         size_t const n_frames = data_size / m_bytes_per_frame;
         // update number of frames in header (first element of shape)
         m_header.shape[0] = n_frames;
-        if (fseek(fp, 0, SEEK_SET)) {
+        if (fseek64(fp, 0, SEEK_SET)) {
             std::cout << "Could not seek to beginning of file" << std::endl;
         }
         // create string stream to contain header

src/ROI.cpp

@@ -20,7 +20,7 @@ bool complete_ROI(const ROI &roi, const DetectorGeometry &geometry) {
 
 bool complete_ROI(const std::vector<ROI> &rois,
                   const DetectorGeometry &geometry) {
-    if (rois.empty() or rois.size() > 1) {
+    if (rois.empty() || rois.size() > 1) {
         return false;
     } else {
         return complete_ROI(rois[0], geometry);
@@ -34,7 +34,7 @@ bool complete_ROI(const ROIGeometry &roi, const DetectorGeometry &geometry) {
 
 bool complete_ROI(const std::vector<ROIGeometry> &rois,
                   const DetectorGeometry &geometry) {
-    if (rois.empty() or rois.size() > 1) {
+    if (rois.empty() || rois.size() > 1) {
         return false;
     } else {
         return complete_ROI(rois[0], geometry);

src/RawFile.cpp

@@ -302,7 +302,7 @@ void RawFile::open_subfiles(const size_t roi_index) {
 
 DetectorHeader RawFile::read_header(const std::filesystem::path &fname) {
     DetectorHeader h{};
-    FILE *fp = fopen(fname.string().c_str(), "r");
+    FILE *fp = fopen(fname.string().c_str(), "rb");
     if (!fp)
         throw std::runtime_error(fmt::format(
             "Could not open file '{}' for frame index 0", fname.string()));

src/fit/Fit.cpp

@@ -268,7 +268,9 @@ void fit_3d(const FitModel<Model> &model, NDView<double, 1> x,
             NDView<double, 3> y, NDView<double, 3> y_err,
             NDView<double, 3> par_out, NDView<double, 3> err_out,
             NDView<double, 2> chi2_out, int n_threads) {
-    constexpr std::size_t npar = Model::npar;
+    // static so the lambda below can use it as an array size without
+    // capturing it, which MSVC otherwise rejects
+    static constexpr std::size_t npar = Model::npar;
     detail::check_fit_3d_shapes<Model>(x, y, y_err, par_out, err_out, chi2_out);
 
     const bool has_errors = (y_err.size() > 0);

src/fit/Fit.test.cpp

@@ -1100,7 +1100,7 @@ TEST_CASE("Samples with a zero error are ignored even when NaN", "[fit]") {
     INFO("minimizer " << name(minimizer));
     const double nan = std::numeric_limits<double>::quiet_NaN();
 
-    constexpr ssize_t n = 10;
+    static constexpr ssize_t n = 10; // static: used inside GENERATE's lambda
     NDArray<double, 1> x({n});
     NDArray<double, 1> y({n});
     NDArray<double, 1> y_err({n}, 1.0);