Skip to content

Commit 6165cd9

Browse files
authored
CPP-812 - Enable warnings for implicit casts and address problems (#292)
* msvc: Enable warnings for implicit casts and address problems * clang: Enable warnings for implicit casts and address problems * `-Wconversion` for GCC is unsustainable; see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40752 for more context
1 parent cf9dddb commit 6165cd9

28 files changed

Lines changed: 110 additions & 76 deletions

cpp-driver/cmake/modules/CppDriver.cmake

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,18 @@ macro(CassConfigureShared prefix)
151151
if("${prefix}" STREQUAL "DSE")
152152
set(STATIC_COMPILE_FLAGS "${STATIC_COMPILE_FLAGS} -DCASS_BUILDING")
153153
endif()
154-
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR
155-
"${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
154+
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
155+
set_property(
156+
TARGET ${PROJECT_LIB_NAME}
157+
APPEND PROPERTY COMPILE_FLAGS "${STATIC_COMPILE_FLAGS} -Wconversion -Wno-sign-conversion -Wno-shorten-64-to-32 -Wno-undefined-var-template -Werror")
158+
elseif("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") # To many superfluous warnings generated with GCC when using -Wconversion (see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40752)
156159
set_property(
157160
TARGET ${PROJECT_LIB_NAME}
158161
APPEND PROPERTY COMPILE_FLAGS "${STATIC_COMPILE_FLAGS} -Werror")
162+
elseif("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC")
163+
set_property(
164+
TARGET ${PROJECT_LIB_NAME}
165+
APPEND PROPERTY COMPILE_FLAGS "${STATIC_COMPILE_FLAGS} /we4800")
159166
endif()
160167
endmacro()
161168

@@ -181,11 +188,18 @@ macro(CassConfigureStatic prefix)
181188
if("${prefix}" STREQUAL "DSE")
182189
set(STATIC_COMPILE_FLAGS "${STATIC_COMPILE_FLAGS} -DCASS_STATIC")
183190
endif()
184-
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR
185-
"${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
191+
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
192+
set_property(
193+
TARGET ${PROJECT_LIB_NAME_STATIC}
194+
APPEND PROPERTY COMPILE_FLAGS "${STATIC_COMPILE_FLAGS} -Wconversion -Wno-sign-conversion -Wno-shorten-64-to-32 -Wno-undefined-var-template -Werror")
195+
elseif("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU") # To many superfluous warnings generated with GCC when using -Wconversion (see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40752)
186196
set_property(
187197
TARGET ${PROJECT_LIB_NAME_STATIC}
188198
APPEND PROPERTY COMPILE_FLAGS "${STATIC_COMPILE_FLAGS} -Werror")
199+
elseif("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC")
200+
set_property(
201+
TARGET ${PROJECT_LIB_NAME_STATIC}
202+
APPEND PROPERTY COMPILE_FLAGS "${STATIC_COMPILE_FLAGS} /we4800")
189203
endif()
190204

191205
# Update the CXX flags to indicate the use of the static library
@@ -874,7 +888,6 @@ macro(CassSetCompilerFlags)
874888
# TODO(mpenick): Fix these "possible loss of data" warnings
875889
add_definitions(/wd4244)
876890
add_definitions(/wd4267)
877-
add_definitions(/wd4800) # Performance warning due to automatic compiler casting from int to bool
878891

879892
# Add preprocessor definitions for proper compilation
880893
add_definitions(-D_CRT_SECURE_NO_WARNINGS) # Remove warnings for not using safe functions (TODO: Fix codebase to be more secure for Visual Studio)

cpp-driver/src/batch_request.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ int BatchRequest::encode(ProtocolVersion version, RequestCallback* callback,
130130
Buffer buf(buf_size);
131131

132132
size_t pos = buf.encode_byte(0, type_);
133-
buf.encode_uint16(pos, statements().size());
133+
buf.encode_uint16(pos, static_cast<uint16_t>(statements().size()));
134134

135135
bufs->push_back(buf);
136136
length += buf_size;
@@ -183,7 +183,7 @@ int BatchRequest::encode(ProtocolVersion version, RequestCallback* callback,
183183
if (version >= CASS_PROTOCOL_VERSION_V5) {
184184
pos = buf.encode_int32(pos, flags);
185185
} else {
186-
pos = buf.encode_byte(pos, flags);
186+
pos = buf.encode_byte(pos, static_cast<uint8_t>(flags));
187187
}
188188

189189
if (callback->serial_consistency() != 0) {
@@ -195,7 +195,7 @@ int BatchRequest::encode(ProtocolVersion version, RequestCallback* callback,
195195
}
196196

197197
if (version.supports_set_keyspace() && !keyspace().empty()) {
198-
pos = buf.encode_string(pos, keyspace().data(), keyspace().size());
198+
pos = buf.encode_string(pos, keyspace().data(), static_cast<uint16_t>(keyspace().size()));
199199
}
200200

201201
bufs->push_back(buf);

cpp-driver/src/callback.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class Callback {
6565
return *this;
6666
}
6767

68-
operator bool() const { return invoker_; }
68+
operator bool() const { return invoker_ != NULL; }
6969

7070
R operator()(const Arg& arg) const { return invoker_->invoke(arg); }
7171

cpp-driver/src/cluster_config.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ CassError cass_cluster_set_use_hostname_resolution(CassCluster* cluster, cass_bo
408408

409409
CassError cass_cluster_set_use_randomized_contact_points(CassCluster* cluster,
410410
cass_bool_t enabled) {
411-
cluster->config().set_use_randomized_contact_points(enabled);
411+
cluster->config().set_use_randomized_contact_points(enabled == cass_true);
412412
return CASS_OK;
413413
}
414414

cpp-driver/src/data_type.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,13 @@ CassDataType* cass_data_type_new_from_existing(const CassDataType* data_type) {
7272
}
7373

7474
CassDataType* cass_data_type_new_tuple(size_t item_count) {
75-
DataType* data_type = new CollectionType(CASS_VALUE_TYPE_TUPLE, item_count);
75+
DataType* data_type = new CollectionType(CASS_VALUE_TYPE_TUPLE, item_count, false);
7676
data_type->inc_ref();
7777
return CassDataType::to(data_type);
7878
}
7979

8080
CassDataType* cass_data_type_new_udt(size_t field_count) {
81-
DataType* data_type = new UserType(field_count);
81+
DataType* data_type = new UserType(field_count, false);
8282
data_type->inc_ref();
8383
return CassDataType::to(data_type);
8484
}

cpp-driver/src/data_type.hpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -192,15 +192,15 @@ class CollectionType : public CompositeType {
192192
public:
193193
typedef SharedRefPtr<const CollectionType> ConstPtr;
194194

195-
CollectionType(CassValueType collection_type, bool is_frozen)
195+
explicit CollectionType(CassValueType collection_type, bool is_frozen)
196196
: CompositeType(collection_type, is_frozen) {}
197197

198-
CollectionType(CassValueType collection_type, size_t types_count, bool is_frozen)
198+
explicit CollectionType(CassValueType collection_type, size_t types_count, bool is_frozen)
199199
: CompositeType(collection_type, is_frozen) {
200200
types_.reserve(types_count);
201201
}
202202

203-
CollectionType(CassValueType collection_type, const DataType::Vec& types, bool is_frozen)
203+
explicit CollectionType(CassValueType collection_type, const DataType::Vec& types, bool is_frozen)
204204
: CompositeType(collection_type, types, is_frozen) {}
205205

206206
virtual bool equals(const DataType::ConstPtr& data_type) const {
@@ -310,19 +310,20 @@ class UserType : public DataType {
310310

311311
typedef CaseInsensitiveHashTable<Field>::EntryVec FieldVec;
312312

313-
UserType(bool is_frozen)
313+
explicit UserType(bool is_frozen)
314314
: DataType(CASS_VALUE_TYPE_UDT, is_frozen) {}
315315

316-
UserType(size_t field_count, bool is_frozen)
316+
explicit UserType(size_t field_count, bool is_frozen)
317317
: DataType(CASS_VALUE_TYPE_UDT, is_frozen)
318318
, fields_(field_count) {}
319319

320-
UserType(const String& keyspace, const String& type_name, bool is_frozen)
320+
explicit UserType(const String& keyspace, const String& type_name, bool is_frozen)
321321
: DataType(CASS_VALUE_TYPE_UDT, is_frozen)
322322
, keyspace_(keyspace)
323323
, type_name_(type_name) {}
324324

325-
UserType(const String& keyspace, const String& type_name, const FieldVec& fields, bool is_frozen)
325+
explicit UserType(const String& keyspace, const String& type_name, const FieldVec& fields,
326+
bool is_frozen)
326327
: DataType(CASS_VALUE_TYPE_UDT, is_frozen)
327328
, keyspace_(keyspace)
328329
, type_name_(type_name)

cpp-driver/src/dc_aware_policy.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ QueryPlan* DCAwarePolicy::new_query_plan(const String& keyspace, RequestHandler*
9292

9393
bool DCAwarePolicy::is_host_up(const Address& address) const {
9494
ScopedReadLock rl(&available_rwlock_);
95-
return available_.count(address);
95+
return available_.count(address) > 0;
9696
}
9797

9898
void DCAwarePolicy::on_host_added(const Host::Ptr& host) {

cpp-driver/src/decoder.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ bool Decoder::decode_inet(CassInet* output) {
7878
}
7979

8080
bool Decoder::as_inet(const int address_length, CassInet* output) const {
81-
output->address_length = address_length;
81+
output->address_length = static_cast<uint8_t>(address_length);
8282
if (output->address_length > CASS_INET_V6_LENGTH) {
8383
LOG_ERROR("Invalid inet address length of %d bytes", output->address_length);
8484
return false;

cpp-driver/src/encode.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ namespace datastax { namespace internal { namespace core {
2323
static char* encode_vint(char* output, uint64_t value, size_t value_size) {
2424
if (value_size == 1) {
2525
// This is just a one byte value; write it and get out.
26-
*output = value;
26+
*output = static_cast<char>(value);
2727
return output + 1;
2828
}
2929

cpp-driver/src/execute_request.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ int ExecuteRequest::encode(ProtocolVersion version, RequestCallback* callback,
4040
length += bufs->back().size();
4141
}
4242
}
43-
length += encode_begin(version, elements().size(), callback, bufs);
43+
length += encode_begin(version, static_cast<uint16_t>(elements().size()), callback, bufs);
4444
int32_t result = encode_values(version, callback, bufs);
4545
if (result < 0) return result;
4646
length += result;

0 commit comments

Comments
 (0)