From 89438220464778d167d86bb59e095a85ccdba080 Mon Sep 17 00:00:00 2001 From: David Williams Date: Sun, 20 Oct 2013 09:12:37 +0200 Subject: [PATCH 1/3] Added asserts to catch invalid material/density values. --- .../include/PolyVoxCore/MaterialDensityPair.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h b/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h index cdde07f7..9f26b286 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h @@ -75,8 +75,19 @@ namespace PolyVox Type getDensity() const { return m_uDensity; } Type getMaterial() const { return m_uMaterial; } - void setDensity(Type uDensity) { m_uDensity = uDensity; } - void setMaterial(Type uMaterial) { m_uMaterial = uMaterial; } + void setDensity(Type uDensity) + { + POLYVOX_ASSERT(uDensity >= getMinDensity(), "Density out of range"); + POLYVOX_ASSERT(uDensity <= getMaxDensity(), "Density out of range"); + m_uDensity = uDensity; + } + + void setMaterial(Type uMaterial) + { + POLYVOX_ASSERT(uMaterial >= 0, "Material out of range"); + POLYVOX_ASSERT(uMaterial <= (0x01 << NoOfMaterialBits) - 1, "Material out of range"); + m_uMaterial = uMaterial; + } static Type getMaxDensity() { return (0x01 << NoOfDensityBits) - 1; } static Type getMinDensity() { return 0; } From 3fe92086f1bdde06eb3e9bd812fc19a279154458 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Wed, 23 Oct 2013 16:33:42 +0200 Subject: [PATCH 2/3] Attempting to work around a GCC warning. --- .../include/PolyVoxCore/MaterialDensityPair.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h b/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h index 9f26b286..441318ba 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h @@ -77,15 +77,19 @@ namespace PolyVox void setDensity(Type uDensity) { - POLYVOX_ASSERT(uDensity >= getMinDensity(), "Density out of range"); - POLYVOX_ASSERT(uDensity <= getMaxDensity(), "Density out of range"); + // Validate range of parameters. The casts are used to bypass a GCC warning 'comparison + // is always true due to limited range of data type' which is not helpful in this case. + POLYVOX_ASSERT(static_cast(uDensity) >= static_cast(getMinDensity()), "Density out of range"); + POLYVOX_ASSERT(static_cast(uDensity) <= static_cast(getMaxDensity()), "Density out of range"); m_uDensity = uDensity; } void setMaterial(Type uMaterial) { - POLYVOX_ASSERT(uMaterial >= 0, "Material out of range"); - POLYVOX_ASSERT(uMaterial <= (0x01 << NoOfMaterialBits) - 1, "Material out of range"); + // Validate range of parameters. The casts are used to bypass a GCC warning 'comparison + // is always true due to limited range of data type' which is not helpful in this case. + POLYVOX_ASSERT(static_cast(uMaterial) >= static_cast(0), "Material out of range"); + POLYVOX_ASSERT(static_cast(uMaterial) <= static_cast((0x01 << NoOfMaterialBits) - 1), "Material out of range"); m_uMaterial = uMaterial; } From afddb59d6911c14cfb4035eb493c59b077604d16 Mon Sep 17 00:00:00 2001 From: Daviw Williams Date: Thu, 24 Oct 2013 14:37:18 +0200 Subject: [PATCH 3/3] Reverted attempt to avoid warnings. Just suppressed them instead. Revert "Attempting to work around a GCC warning." This reverts commit 3fe92086f1bdde06eb3e9bd812fc19a279154458. --- .../include/PolyVoxCore/Impl/ErrorHandling.h | 17 ++++++++++++- .../include/PolyVoxCore/MaterialDensityPair.h | 24 ++++++++++++------- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h b/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h index 5c6f6b0f..b6c656eb 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h +++ b/library/PolyVoxCore/include/PolyVoxCore/Impl/ErrorHandling.h @@ -48,7 +48,12 @@ freely, subject to the following restrictions: // Macros cannot contain #ifdefs, but some of our macros need to disable warnings and such warning supression is // platform specific. But macros can contain other macros, so we create macros to control the warnings and use -// those instead. This set of warning supression macros can be extended to GCC/Clang when required. +// those instead. +// +// Note that we have seperate macros for POLYVOX_MSC_..., POLYVOX_GCC_..., etc. In princpiple we could have just one +// as compilers should ignore pragmas they don't recognise, but in practice at least MSVC complains about this as +// well. So in practice it's just eaier to have seperate macros. We could look into the compiler switch to not warn +// on unrecognised pragmas though. #if defined(_MSC_VER) #define POLYVOX_MSC_WARNING_PUSH __pragma(warning(push)) #define POLYVOX_DISABLE_MSC_WARNING(x) __pragma(warning(disable:x)) @@ -59,6 +64,16 @@ freely, subject to the following restrictions: #define POLYVOX_MSC_WARNING_POP #endif +#if defined(__GNUC__) + #define POLYVOX_GCC_WARNING_PUSH #pragma GCC diagnostic push + #define POLYVOX_DISABLE_GCC_WARNING(x) #pragma GCC diagnostic ignored x + #define POLYVOX_GCC_WARNING_POP #pragma GCC diagnostic pop +#else + #define POLYVOX_GCC_WARNING_PUSH + #define POLYVOX_DISABLE_GCC_WARNING(x) + #define POLYVOX_GCC_WARNING_POP +#endif + #define POLYVOX_UNUSED(x) do { (void)sizeof(x); } while(0) /* diff --git a/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h b/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h index 441318ba..a051f74e 100644 --- a/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h +++ b/library/PolyVoxCore/include/PolyVoxCore/MaterialDensityPair.h @@ -77,19 +77,27 @@ namespace PolyVox void setDensity(Type uDensity) { - // Validate range of parameters. The casts are used to bypass a GCC warning 'comparison - // is always true due to limited range of data type' which is not helpful in this case. - POLYVOX_ASSERT(static_cast(uDensity) >= static_cast(getMinDensity()), "Density out of range"); - POLYVOX_ASSERT(static_cast(uDensity) <= static_cast(getMaxDensity()), "Density out of range"); + // Depending on our underlying type it may be impossible for the assert below to be triggered (i.e. if density is stored as + // Type, rather than just using a few bits of Type). GCC will warn about this but it's redundant so we diable the warning. + POLYVOX_GCC_WARNING_PUSH + POLYVOX_DISABLE_GCC_WARNING("-Wtype-limits") + POLYVOX_ASSERT(uDensity >= getMinDensity(), "Density out of range"); + POLYVOX_ASSERT(uDensity <= getMaxDensity(), "Density out of range"); + POLYVOX_GCC_WARNING_POP + m_uDensity = uDensity; } void setMaterial(Type uMaterial) { - // Validate range of parameters. The casts are used to bypass a GCC warning 'comparison - // is always true due to limited range of data type' which is not helpful in this case. - POLYVOX_ASSERT(static_cast(uMaterial) >= static_cast(0), "Material out of range"); - POLYVOX_ASSERT(static_cast(uMaterial) <= static_cast((0x01 << NoOfMaterialBits) - 1), "Material out of range"); + // Depending on our underlying type it may be impossible for the assert below to be triggered (i.e. if material is stored as + // Type, rather than just using a few bits of Type). GCC will warn about this but it's redundant so we diable the warning. + POLYVOX_GCC_WARNING_PUSH + POLYVOX_DISABLE_GCC_WARNING("-Wtype-limits") + POLYVOX_ASSERT(uMaterial >= 0, "Material out of range"); + POLYVOX_ASSERT(uMaterial <= (0x01 << NoOfMaterialBits) - 1, "Material out of range"); + POLYVOX_GCC_WARNING_POP + m_uMaterial = uMaterial; }