[c++] Fix GCC warnings in release build

The major classes of warnings were
* potentially uninitialized variables,
* violations of strict aliasing rules, and
* unused variables.

Potentially uninitialized variables were set to sensible defaults.

The strict aliasing violations stemmed from type punning. They were
fixed by eliminating the type punning or switching to using
`std::memcpy` to perform type punning.

Unused variable warnings stemmed from our tests and examples using
`assert()` for the check. Since `assert()` is eliminated in a release
build, these warnings were fixed by using `boost::ignore_unused`.
However, the use of `assert()` for testing makes the "check" build
target much less useful for a release build. Issue
https://github.com/Microsoft/bond/issues/291 is open to improve this.
Until then, `boost::ignore_unused`.

When initializing SchemaDef, there was a place where we were casting a
dummy `char` to a `T&`. Since the `T&` is never referenced when creating
the SchemaDef, we now create a `const T&` from `nullptr`.

Closes https://github.com/Microsoft/bond/pull/292
This commit is contained in:
Christopher Warrington 2017-01-05 11:35:42 -08:00
Родитель 73d7cdfe12
Коммит bc4f7d881a
8 изменённых файлов: 76 добавлений и 71 удалений

Просмотреть файл

@ -298,8 +298,9 @@ void SchemaCache<T, Unused>::AppendStructDef(SchemaDef* s)
// need to support stateful allocators that can't allocate from
// a default-constructed instance and containers which allocate in
// default constructor.
char o;
Apply(InitSchemaDef(*s), reinterpret_cast<const T&>(o));
//
// Thus, we make a dummy const T& from a nullptr.
Apply(InitSchemaDef(*s), static_cast<const T&>(*static_cast<T*>(0)));
}
} // detail

Просмотреть файл

@ -248,7 +248,7 @@ private:
typename boost::enable_if<is_basic_type<T> >::type
Write(const value<T, Reader>& value) const
{
T data;
T data = T();
value.Deserialize(data);
Write(data);

Просмотреть файл

@ -734,7 +734,7 @@ typename boost::disable_if<is_container<X> >::type
inline DeserializeContainer(X& var, const T& element, Reader& input)
{
BondDataType type = GetTypeId(element);
uint32_t size;
uint32_t size = 0;
input.ReadContainerBegin(size, type);
@ -901,7 +901,7 @@ typename boost::disable_if<is_container<X> >::type
inline DeserializeMap(X& var, BondDataType keyType, const T& element, Reader& input)
{
std::pair<BondDataType, BondDataType> type(keyType, GetTypeId(element));
uint32_t size;
uint32_t size = 0;
input.ReadContainerBegin(size, type);

Просмотреть файл

@ -5,6 +5,10 @@
#include <bond/core/containers.h>
#include <bond/core/blob.h>
#include <bond/core/traits.h>
#include <cstring>
#include <boost/static_assert.hpp>
namespace bond
{
@ -14,7 +18,7 @@ class RandomProtocolEngine
{
public:
// We don't need a good pseudo-random number generator but we do need one
// that is consistent accross compilers/libraries so that we can use
// that is consistent accross compilers/libraries so that we can use
// randomly generated data files from one platfom to verify compatibility
// on another platform.
RandomProtocolEngine()
@ -46,11 +50,11 @@ template <typename T>
uint64_t RandomProtocolEngine<T>::state[2] = {seed1, seed2};
//
// Protocol which generates a random stream of bits.
// Protocol which generates a random stream of bits.
// In some cases it may be useful for initializing data in tests, e.g.:
//
// Params param;
// Apply(bond::To<Params>(param), bond::bonded<Params>(bond::RandomProtocolReader()));
// Apply(bond::To<Params>(param), bond::bonded<Params>(bond::RandomProtocolReader()));
//
class RandomProtocolReader
: public RandomProtocolEngine<RandomProtocolReader>
@ -59,7 +63,7 @@ public:
typedef bond::StaticParser<RandomProtocolReader&> Parser;
RandomProtocolReader(uint32_t max_string_length = 50, uint32_t max_list_size = 20, bool json = false)
: _max_string_length(max_string_length),
: _max_string_length(max_string_length),
_max_list_size(max_list_size),
_json(json)
{}
@ -76,11 +80,19 @@ public:
template <typename T>
typename boost::disable_if<is_string_type<T> >::type
Read(T& value)
Read(T& value)
{
uint64_t random = RandomProtocolEngine::Next();
// T needs to be trivially copyable to use std::memcpy.
// std::is_trivially_copyable isn't available until C++11, so we use
// bond::is_arithmetic instead. This works, because we only ever
// read arithmetic types in this version of Read(), so we don't need
// the full generality.
BOOST_STATIC_ASSERT(bond::is_arithmetic<T>::value);
// We only have 64 bits of randomness, so T needs to fit in that.
BOOST_STATIC_ASSERT(sizeof(T) <= sizeof(uint64_t));
value = *static_cast<T*>(static_cast<void*>(&random));
uint64_t random = RandomProtocolEngine::Next();
std::memcpy(&value, &random, sizeof(T));
}
void Read(uint64_t& value)
@ -145,7 +157,7 @@ public:
Read(T& value)
{
uint32_t length = 0;
Read(length);
length %= _max_string_length;
@ -169,7 +181,7 @@ public:
void Read(blob& value, uint32_t size)
{
boost::shared_ptr<char[]> buffer(boost::make_shared_noinit<char[]>(size));
for (unsigned i = 0; i < size; ++i)
Read(buffer[i]);
@ -204,11 +216,11 @@ private:
};
template <typename Unused> struct
template <typename Unused> struct
uses_marshaled_bonded<RandomProtocolReader, Unused>
: boost::false_type {};
template <typename Unused> struct
template <typename Unused> struct
uses_marshaled_bonded<RandomProtocolReader&, Unused>
: boost::false_type {};

Просмотреть файл

@ -1,23 +1,18 @@
#pragma once
#include <algorithm>
#include <complex>
#include <limits>
template <typename T>
class Compare;
inline bool Equal(double left, double right)
{
// http://www.cygnus-software.com/papers/comparingfloats/comparingfloats.htm
int64_t l = *(int64_t*)&left;
if (l < 0)
l = 0x8000000000000000LL - l;
int64_t r = *(int64_t*)&right;
if (r < 0)
r = 0x8000000000000000LL - r;
return (l - r) < 5 && (l - r) > -5;
const int ulp = 5;
return std::abs(left - right) <= std::numeric_limits<double>::epsilon() * (std::max)(std::abs(left), std::abs(right)) * ulp;
}
template <typename T>
typename boost::enable_if<bond::is_basic_type<T>, bool>::type
Equal(const T& left, const T& right)
@ -75,7 +70,7 @@ Equal(const T& left, const T& right)
template <typename T1, typename T2>
bool Equal(const std::pair<T1, T2>& left, const std::pair<T1, T2>& right)
{
return Equal(left.first, right.first)
return Equal(left.first, right.first)
&& Equal(left.second, right.second);
}

Просмотреть файл

@ -2,6 +2,10 @@
#include <bond/core/reflection.h>
#include <algorithm>
#include <complex>
#include <limits>
#define UT_Equal(x, y) UT_AssertIsTrue(Equal(x, y))
template <typename T1, typename T2>
@ -14,7 +18,7 @@ template <typename T1, typename T2>
bool operator==(const BondStruct<T1>& left, const BondStruct<T2>& right);
template <typename T1, typename T2>
typename boost::enable_if<bond::is_container<T1>, bool>::type
typename boost::enable_if<bond::is_container<T1>, bool>::type
Equal(const T1& lhs, const T2& rhs);
template <typename T>
@ -41,16 +45,8 @@ private:
inline bool Equal(double left, double right)
{
// http://www.cygnus-software.com/papers/comparingfloats/comparingfloats.htm
int64_t l = *(int64_t*)&left;
if (l < 0)
l = 0x8000000000000000LL - l;
int64_t r = *(int64_t*)&right;
if (r < 0)
r = 0x8000000000000000LL - r;
return (l - r) < 5 && (l - r) > -5;
const int ulp = 5;
return std::abs(left - right) <= std::numeric_limits<double>::epsilon() * (std::max)(std::abs(left), std::abs(right)) * ulp;
}
@ -71,7 +67,7 @@ Equal(const T& left, const T& right)
// "loose" equality for matching but different types
template <typename T1, typename T2>
typename boost::enable_if<bond::is_basic_type<T1>, bool>::type
typename boost::enable_if<bond::is_basic_type<T1>, bool>::type
Equal(const T1& lhs, const T2& rhs)
{
return bond::cast<T2>(lhs) == rhs;
@ -80,8 +76,8 @@ Equal(const T1& lhs, const T2& rhs)
// "loose" equality for struct just calls ==
template <typename T1, typename T2>
typename boost::enable_if_c<bond::has_schema<T1>::value
&& bond::has_schema<T2>::value, bool>::type
typename boost::enable_if_c<bond::has_schema<T1>::value
&& bond::has_schema<T2>::value, bool>::type
Equal(const T1& lhs, const T2& rhs)
{
return lhs == rhs;
@ -143,7 +139,7 @@ bool Equal(const std::pair<T1, T2>& p1, const std::pair<T1, T2>& p2)
// "loose" equality for lists of matching but different types
template <typename T1, typename T2>
typename boost::enable_if<bond::is_container<T1>, bool>::type
typename boost::enable_if<bond::is_container<T1>, bool>::type
Equal(const T1& lhs, const T2& rhs)
{
if (container_size(lhs) != container_size(rhs))
@ -177,7 +173,7 @@ inline bool Equal(const std::vector<int8_t>& lhs, const bond::blob& rhs)
template <typename T1, typename T2>
typename boost::enable_if_c<bond::has_base<T1>::value
typename boost::enable_if_c<bond::has_base<T1>::value
&& bond::has_base<T2>::value, bool>::type
BaseIsEqual(const T1& left, const T2& right)
{
@ -276,4 +272,3 @@ void Compare<T>::operator()(const Field&)
{
equal = equal && Equal(Field::GetVariable(left), Field::GetVariable(right));
}

Просмотреть файл

@ -2,68 +2,70 @@
#include <bond/core/validate.h>
template <typename T> struct
#include <boost/static_assert.hpp>
template <typename T> struct
compatible_types
{
typedef boost::mpl::list<> type;
};
// uint32 -> uint64
template <> struct
template <> struct
compatible_types<uint32_t>
{
typedef boost::mpl::list<uint64_t> type;
};
// uint16 -> uint32
template <> struct
template <> struct
compatible_types<uint16_t>
{
typedef boost::mpl::list<uint64_t, uint32_t> type;
};
// uint8 -> uint16
template <> struct
template <> struct
compatible_types<uint8_t>
{
typedef boost::mpl::list<uint64_t, uint32_t, uint16_t> type;
};
// int32 -> int64
template <> struct
template <> struct
compatible_types<int32_t>
{
typedef boost::mpl::list<int64_t> type;
};
// int16 -> int32
template <> struct
template <> struct
compatible_types<int16_t>
{
typedef boost::mpl::list<int64_t, int32_t> type;
};
// int8 -> int16
template <> struct
template <> struct
compatible_types<int8_t>
{
typedef boost::mpl::list<int64_t, int32_t, int16_t> type;
};
// float -> double
template <> struct
template <> struct
compatible_types<float>
{
typedef boost::mpl::list<double> type;
};
template <typename T> struct
template <typename T> struct
compatible_types_or_self
{
typedef typename boost::mpl::push_front<typename compatible_types<T>::type, T>::type type;
};
template <typename T> struct
template <typename T> struct
incompatible_types
{
typedef typename boost::mpl::remove_if<NumericTypes, boost::mpl::contains<typename compatible_types_or_self<T>::type, _> >::type type;
@ -74,8 +76,8 @@ inline bool ValidateCompatible()
{
bond::RuntimeSchema src = bond::GetRuntimeSchema<T1>();
bond::RuntimeSchema dst = bond::GetRuntimeSchema<T2>();
try
try
{
bond::Validate(src, dst);
return true;
@ -93,6 +95,8 @@ struct NumericConverterFail
void operator()(const Num2&)
{
BOOST_STATIC_ASSERT(!(bond::is_matching_basic<Num1, Num2>::value));
BOOST_STATIC_ASSERT(boost::is_arithmetic<Num1>::value || boost::is_enum<Num1>::value);
BOOST_STATIC_ASSERT(boost::is_arithmetic<Num2>::value || boost::is_enum<Num2>::value);
typedef BondStruct<Num1> From;
typedef BondStruct<Num2> To;
@ -100,20 +104,17 @@ struct NumericConverterFail
// Num1 and Num2 are some incompatible types.
// In order to "compare" them, we are setting one to zero
// and the other to non-zero.
UT_AssertIsFalse(ValidateCompatible<From, To>());
From from;
from.field = static_cast<Num1>(0);
bond::bonded<From> bonded(GetBonded<Reader, Writer, From>(from));
bond::bonded<void> bonded_void(bonded);
To to;
static uint64_t nonZero = static_cast<uint64_t>(-1);
to.field = static_cast<Num2>(1);
to.field = *static_cast<Num2*>(static_cast<void*>(&nonZero));
UT_AssertIsTrue(!!to.field);
UT_AssertIsFalse(!!from.field);
@ -121,7 +122,7 @@ struct NumericConverterFail
UT_AssertIsTrue(!!to.field);
bonded_void.Deserialize(to);
UT_AssertIsTrue(!!to.field);
UT_AssertIsTrue(!!to.field);
}
};
@ -155,7 +156,7 @@ struct NumericConverter2
{
typedef BondStruct<list<Num1> > From;
typedef BondStruct<vector<Num2> > To;
AllBindingAndMapping<Reader, Writer, From, To>();
UT_AssertIsTrue(ValidateCompatible<From, To>());
}
@ -163,7 +164,7 @@ struct NumericConverter2
{
typedef BondStruct<bond::nullable<Num1> > From;
typedef BondStruct<bond::nullable<Num2> > To;
AllBindingAndMapping<Reader, Writer, From, To>();
UT_AssertIsTrue(ValidateCompatible<From, To>());
}
@ -171,7 +172,7 @@ struct NumericConverter2
{
typedef BondStruct<std::set<Num1> > From;
typedef BondStruct<std::set<Num2> > To;
AllBindingAndMapping<Reader, Writer, From, To>();
UT_AssertIsTrue(ValidateCompatible<From, To>());
}
@ -179,7 +180,7 @@ struct NumericConverter2
{
typedef BondStruct<std::map<Num1, uint8_t> > From;
typedef BondStruct<std::map<Num2, uint32_t> > To;
AllBindingAndMapping<Reader, Writer, From, To>();
UT_AssertIsTrue(ValidateCompatible<From, To>());
}
@ -187,12 +188,9 @@ struct NumericConverter2
{
typedef BondStruct<std::map<int16_t, Num1> > From;
typedef BondStruct<std::map<int64_t, Num2> > To;
AllBindingAndMapping<Reader, Writer, From, To>();
UT_AssertIsTrue(ValidateCompatible<From, To>());
}
}
};

Просмотреть файл

@ -2,6 +2,7 @@
#include <limits>
#include <stdint.h>
#include <bond/core/exception.h>
#include <boost/core/ignore_unused.hpp>
#include "enumerations_enum.h"
#include "enumerations_types.h"
@ -33,12 +34,14 @@ void DisambiguateEnumsWithSameName()
color = Color::Orange;
color = Yellow;
boost::ignore_unused(color);
assert(color == Yellow);
Fruit fruit;
fruit = Apple;
fruit = Fruit::Orange;
boost::ignore_unused(fruit);
assert(fruit == Fruit::Orange);
}
@ -59,6 +62,7 @@ void ConversionToFromEnum()
{
std::string name;
bool result = FromEnum(name, Yellow);
boost::ignore_unused(result);
assert(result);
assert(name == "Yellow");