From b4710027e9faaa9f194272426ab5a100e24e7efa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Sch=C3=A4fer?= Date: Wed, 1 Mar 2017 10:43:20 +0100 Subject: [PATCH] implement various fixes for warnings issued by MSVC, re #49 --- src/examples/hardwarefluke/main.cpp | 14 +++--- src/libgeodecomp/geometry/region.h | 29 ++++++++---- src/libgeodecomp/io/simplecellplotter.h | 2 +- .../io/test/parallel_mpi_1/plottertest.h | 2 +- .../io/test/parallel_mpi_1/ppmwritertest.h | 2 +- .../test/parallel_mpi_1/qtwidgetwritertest.h | 2 +- src/libgeodecomp/misc/color.h | 45 +++++++++++++------ src/libgeodecomp/misc/quickpalette.h | 8 ++-- .../storage/test/unit/selectortest.h | 4 +- .../storage/updatefunctormacrosmsvc.h | 30 +++++++------ 10 files changed, 85 insertions(+), 53 deletions(-) diff --git a/src/examples/hardwarefluke/main.cpp b/src/examples/hardwarefluke/main.cpp index d0324a59a..0d5f17cb0 100644 --- a/src/examples/hardwarefluke/main.cpp +++ b/src/examples/hardwarefluke/main.cpp @@ -20,11 +20,11 @@ class BuggyCell {} template - void update(const COORD_MAP& neighbors, unsigned nanoStep) + void update(const COORD_MAP& neighbors, unsigned /* nanoStep */) { int buf = (neighbors[Coord<2>(0, -1)].val + neighbors[Coord<2>(-1, 0)].val + - neighbors[Coord<2>(1, 0)].val + neighbors[Coord<2>( 0, 1)].val); - val = (buf >> 2) + 1; + neighbors[Coord<2>(1, 0)].val + neighbors[Coord<2>( 0, 1)].val); + val = char((buf >> 2) + 1); } private: @@ -34,7 +34,7 @@ class BuggyCell class BuggyCellInitializer : public SimpleInitializer { public: - explicit BuggyCellInitializer(std::string infile, unsigned steps=10000) : + explicit BuggyCellInitializer(std::string infile, unsigned steps = 10000) : SimpleInitializer(readDim(infile), steps), filename(infile) {} @@ -88,9 +88,9 @@ class BuggyCellToColor public: Color operator[](char val) const { - unsigned char r = ((val >> 5) & 7) * 255 / 7; - unsigned char g = ((val >> 2) & 7) * 255 / 7; - unsigned char b = ((val >> 0) & 3) * 255 / 3; + int r = ((val >> 5) & 7) * 255 / 7; + int g = ((val >> 2) & 7) * 255 / 7; + int b = ((val >> 0) & 3) * 255 / 3; return Color(r, g, b); } }; diff --git a/src/libgeodecomp/geometry/region.h b/src/libgeodecomp/geometry/region.h index 7bee02f6c..b97453102 100644 --- a/src/libgeodecomp/geometry/region.h +++ b/src/libgeodecomp/geometry/region.h @@ -151,6 +151,13 @@ class ConstructStreakFromIterators<0> } }; +// Hardwire this warning to off as MSVC would otherwise complain about +// an assignment operator missing -- which is clearly there: +#ifdef _MSC_BUILD +#pragma warning( push ) +#pragma warning( disable : 4626 ) +#endif + /** * internal helper class */ @@ -165,6 +172,11 @@ class StreakIteratorInitPlaneOffset offset(offset) {} +#ifdef LIBGEODECOMP_WITH_CPP14 + inline StreakIteratorInitPlaneOffset(const StreakIteratorInitPlaneOffset& other) = default; + inline StreakIteratorInitPlaneOffset(StreakIteratorInitPlaneOffset&& other) = default; +#endif + template inline void operator()( Streak *streak, @@ -173,9 +185,10 @@ class StreakIteratorInitPlaneOffset const Coord& unusedOffsets, int unusedAdditionalLength) const { - iterators[DIM] = region.indicesBegin(DIM) + offset; + iterators[DIM] = region.indicesBegin(DIM) + std::ptrdiff_t(offset); for (int d = DIM - 1; d >= 0; --d) { - iterators[d] = region.indicesBegin(d) + iterators[d + 1]->second; + std::size_t index = std::size_t(d); + iterators[index] = region.indicesBegin(index) + iterators[index + 1]->second; } ConstructStreakFromIterators()(streak, iterators, unusedOffsets, unusedAdditionalLength); @@ -199,6 +212,11 @@ class StreakIteratorInitSingleOffset offsetIndex(offsetIndex) {} +#ifdef LIBGEODECOMP_WITH_CPP14 + inline StreakIteratorInitSingleOffset(const StreakIteratorInitSingleOffset& other) = default; + inline StreakIteratorInitSingleOffset(StreakIteratorInitSingleOffset&& other) = default; +#endif + template inline std::size_t operator()( Streak *streak, @@ -223,13 +241,6 @@ class StreakIteratorInitSingleOffset const std::size_t offsetIndex; }; -// Hardwire this warning to off as MSVC would otherwise complain about -// an assignment operator missing -- which is clearly there: -#ifdef _MSC_BUILD -#pragma warning( push ) -#pragma warning( disable : 4626 ) -#endif - /** * internal helper class */ diff --git a/src/libgeodecomp/io/simplecellplotter.h b/src/libgeodecomp/io/simplecellplotter.h index 3de288413..2965511d8 100644 --- a/src/libgeodecomp/io/simplecellplotter.h +++ b/src/libgeodecomp/io/simplecellplotter.h @@ -40,7 +40,7 @@ class CellToColor : public Filter void copyStreakInImpl( const Color* /* source */, MemoryLocation::Location sourceLocation, - MEMBER *target, + MEMBER* /* target */, MemoryLocation::Location targetLocation, const std::size_t /* num */, const std::size_t /* stride */) diff --git a/src/libgeodecomp/io/test/parallel_mpi_1/plottertest.h b/src/libgeodecomp/io/test/parallel_mpi_1/plottertest.h index 134827db4..8788bc443 100644 --- a/src/libgeodecomp/io/test/parallel_mpi_1/plottertest.h +++ b/src/libgeodecomp/io/test/parallel_mpi_1/plottertest.h @@ -14,7 +14,7 @@ class TestCellPalette public: Color operator[](const double value) const { - return Color(value, 47, 11); + return Color(value, 47.0, 11.0); } }; diff --git a/src/libgeodecomp/io/test/parallel_mpi_1/ppmwritertest.h b/src/libgeodecomp/io/test/parallel_mpi_1/ppmwritertest.h index 3d51b8979..15967562e 100644 --- a/src/libgeodecomp/io/test/parallel_mpi_1/ppmwritertest.h +++ b/src/libgeodecomp/io/test/parallel_mpi_1/ppmwritertest.h @@ -17,7 +17,7 @@ class TestCellPalette public: Color operator[](const double value) const { - return Color(value, 47, 11); + return Color(value, 47.0, 11.0); } }; diff --git a/src/libgeodecomp/io/test/parallel_mpi_1/qtwidgetwritertest.h b/src/libgeodecomp/io/test/parallel_mpi_1/qtwidgetwritertest.h index cb9701ca8..ea71582f0 100644 --- a/src/libgeodecomp/io/test/parallel_mpi_1/qtwidgetwritertest.h +++ b/src/libgeodecomp/io/test/parallel_mpi_1/qtwidgetwritertest.h @@ -65,7 +65,7 @@ class MyQtCellColorConverter public: Color operator()(const MyQtTestCell& cell) { - return Color(255, cell.temp * 255, 0); + return Color(255.0, cell.temp * 255, 0.0); } }; diff --git a/src/libgeodecomp/misc/color.h b/src/libgeodecomp/misc/color.h index 827851e8c..0326a32b7 100644 --- a/src/libgeodecomp/misc/color.h +++ b/src/libgeodecomp/misc/color.h @@ -28,21 +28,40 @@ class Color unsigned rgb; - inline Color() { *this = Color(0, 0, 0); } + inline Color() : + rgb(255 << 24) + {} inline Color( - const unsigned char& r, - const unsigned char& g, - const unsigned char& b) - { - rgb = 255; - rgb <<= 8; - rgb += r; - rgb <<= 8; - rgb += g; - rgb <<= 8; - rgb += b; - } + const unsigned char r, + const unsigned char g, + const unsigned char b) : + rgb((255 << 24) + (r << 16) + (g << 8) + b) + {} + + inline Color( + const char r, + const char g, + const char b) : + rgb((255 << 24) + + (static_cast(r) << 16) + + (static_cast(g) << 8) + + static_cast(b)) + {} + + inline Color( + const int r, + const int g, + const int b) : + rgb((255 << 24) + (r << 16) + (g << 8) + b) + {} + + inline Color( + const double r, + const double g, + const double b) : + rgb((255 << 24) + (int(r) << 16) + (int(g) << 8) + int(b)) + {} unsigned char red() const { diff --git a/src/libgeodecomp/misc/quickpalette.h b/src/libgeodecomp/misc/quickpalette.h index 04b33e6e1..8a3bc5dab 100644 --- a/src/libgeodecomp/misc/quickpalette.h +++ b/src/libgeodecomp/misc/quickpalette.h @@ -36,18 +36,18 @@ class QuickPalette } if (val < mark1) { - return Color(0, (val - mark0) * mult, 255); + return Color(0.0, (val - mark0) * mult, 255.0); } if (val < mark2) { - return Color(0, 255, (mark2 - val) * mult); + return Color(0.0, 255.0, (mark2 - val) * mult); } if (val < mark3) { - return Color((val - mark2) * mult, 255, 0); + return Color((val - mark2) * mult, 255.0, 0.0); } - return Color(255, (mark4 - val) * mult, 0); + return Color(255.0, (mark4 - val) * mult, 0.0); } private: diff --git a/src/libgeodecomp/storage/test/unit/selectortest.h b/src/libgeodecomp/storage/test/unit/selectortest.h index cc08771ef..d7a08b28b 100644 --- a/src/libgeodecomp/storage/test/unit/selectortest.h +++ b/src/libgeodecomp/storage/test/unit/selectortest.h @@ -173,7 +173,7 @@ class SelectorTest : public CxxTest::TestSuite const std::size_t stride) { for (std::size_t i = 0; i < num; ++i) { - target[i] = Color(source[i], 47, 11); + target[i] = Color(source[i], 47.0, 11.0); } } @@ -199,7 +199,7 @@ class SelectorTest : public CxxTest::TestSuite double MyDummyCell:: *memberPointer) { for (std::size_t i = 0; i < num; ++i) { - target[i] = Color(source[i].*memberPointer, 47, 11); + target[i] = Color(source[i].*memberPointer, 47.0, 11.0); } } }; diff --git a/src/libgeodecomp/storage/updatefunctormacrosmsvc.h b/src/libgeodecomp/storage/updatefunctormacrosmsvc.h index 6da2f5c49..9a9f24c0a 100644 --- a/src/libgeodecomp/storage/updatefunctormacrosmsvc.h +++ b/src/libgeodecomp/storage/updatefunctormacrosmsvc.h @@ -2,7 +2,9 @@ #define LIBGEODECOMP_STORAGE_UPDATEFUNCTORMACROSMSVC_H // MSVC doesn't have a _Pragma(), but requires __pragma(), hence here -// is basically a copy of updatefunctormacros.h: +// is basically a copy of updatefunctormacros.h. Oh, but of course we +// have to shun all std::size_t loop counters as MSVC wouldn't accept +// it as loop counter for OpenMP. #ifdef _MSC_BUILD #ifdef LIBGEODECOMP_WITH_THREADS @@ -10,12 +12,12 @@ if (concurrencySpec.enableOpenMP() && \ !modelThreadingSpec.hasOpenMP()) { \ if (concurrencySpec.preferStaticScheduling()) { \ - __pragma("omp parallel for schedule(static)") \ - for (std::size_t c = 0; c < region.numPlanes(); ++c) { \ + __pragma(omp parallel for schedule(static)) \ + for (int c = 0; c < int(region.numPlanes()); ++c) { \ typename Region::StreakIterator e = \ - region.planeStreakIterator(c + 1); \ + region.planeStreakIterator(std::size_t(c + 1)); \ typedef typename Region::StreakIterator Iter; \ - for (Iter i = region.planeStreakIterator(c + 0); \ + for (Iter i = region.planeStreakIterator(std::size_t(c + 0)); \ i != e; \ ++i) { \ LGD_UPDATE_FUNCTOR_BODY; \ @@ -25,12 +27,12 @@ #define LGD_UPDATE_FUNCTOR_THREADING_SELECTOR_2 \ } else { \ if (!concurrencySpec.preferFineGrainedParallelism()) { \ - __pragma("omp parallel for schedule(dynamic)") \ - for (std::size_t c = 0; c < region.numPlanes(); ++c) { \ + __pragma(omp parallel for schedule(dynamic)) \ + for (int c = 0; c < int(region.numPlanes()); ++c) { \ typename Region::StreakIterator e = \ - region.planeStreakIterator(c + 1); \ + region.planeStreakIterator(std::size_t(c + 1)); \ typedef typename Region::StreakIterator Iter; \ - for (Iter i = region.planeStreakIterator(c + 0); \ + for (Iter i = region.planeStreakIterator(std::size_t(c + 0)); \ i != e; \ ++i) { \ LGD_UPDATE_FUNCTOR_BODY; \ @@ -56,11 +58,11 @@ } \ streaks.push_back(s); \ } \ - __pragma("omp parallel for schedule(dynamic)") \ - for (std::size_t j = 0; j < streaks.size(); ++j) { \ - Streak *i = &streaks[j]; \ - LGD_UPDATE_FUNCTOR_BODY; \ - } \ + __pragma(omp parallel for schedule(dynamic)) \ + for (int j = 0; j < int(streaks.size()); ++j) { \ + Streak *i = &streaks[std::size_t(j)]; \ + LGD_UPDATE_FUNCTOR_BODY; \ + } \ } \ /**/ #define LGD_UPDATE_FUNCTOR_THREADING_SELECTOR_4 \