[C++] Turn on -Wuseless-cast and -Wconversion

lang/c++/CMakeLists.txt

@@ -68,7 +68,7 @@ if (WIN32 AND NOT CYGWIN AND NOT MSYS)
 endif()
 
 if (CMAKE_COMPILER_IS_GNUCXX)
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -pedantic -Werror")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Wuseless-cast -Wconversion -pedantic -Werror")
 if (AVRO_ADD_PROTECTOR_FLAGS)
     set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fstack-protector-all -D_GLIBCXX_DEBUG")
     # Unset _GLIBCXX_DEBUG for avrogencpp.cc because using Boost Program Options

lang/c++/api/LogicalType.hh

@@ -47,17 +47,17 @@ public:
     // Precision must be positive and scale must be either positive or zero. The
     // setters will throw an exception if they are called on any type other
     // than DECIMAL.
-    void setPrecision(int precision);
-    int precision() const { return precision_; }
-    void setScale(int scale);
-    int scale() const { return scale_; }
+    void setPrecision(int32_t precision);
+    int32_t precision() const { return precision_; }
+    void setScale(int32_t scale);
+    int32_t scale() const { return scale_; }
 
     void printJson(std::ostream &os) const;
 
 private:
     Type type_;
-    int precision_;
-    int scale_;
+    int32_t precision_;
+    int32_t scale_;
 };
 
 } // namespace avro

lang/c++/api/NodeImpl.hh

@@ -226,8 +226,8 @@ using LeafNames = concepts::MultiAttribute<std::string>;
 using MultiAttributes = concepts::MultiAttribute<CustomAttributes>;
 using NoAttributes = concepts::NoAttribute<CustomAttributes>;
 
-using NoSize = concepts::NoAttribute<int>;
-using HasSize = concepts::SingleAttribute<int>;
+using NoSize = concepts::NoAttribute<size_t>;
+using HasSize = concepts::SingleAttribute<size_t>;
 
 using NodeImplPrimitive = NodeImpl<NoName, NoLeaves, NoLeafNames, MultiAttributes, NoSize>;
 using NodeImplSymbolic = NodeImpl<HasName, NoLeaves, NoLeafNames, NoAttributes, NoSize>;

lang/c++/api/Reader.hh

@@ -176,15 +176,15 @@ private:
         return encoded;
     }
 
-    int64_t readSize() {
+    size_t readSize() {
         uint64_t encoded = readVarInt();
-        int64_t size = decodeZigzag64(encoded);
+        auto size = static_cast<size_t>(decodeZigzag64(encoded));
         return size;
     }
 
-    int64_t readCount() {
+    size_t readCount() {
         validator_.checkTypeExpected(AVRO_LONG);
-        int64_t count = readSize();
+        size_t count = readSize();
         validator_.setCount(count);
         return count;
     }

lang/c++/api/Validator.hh

@@ -35,7 +35,7 @@ public:
     explicit NullValidator(const ValidSchema &) {}
     NullValidator() = default;
 
-    void setCount(int64_t) {}
+    void setCount(size_t) {}
 
     static bool typeIsExpected(Type) {
         return true;
@@ -45,7 +45,7 @@ public:
         return AVRO_UNKNOWN;
     }
 
-    static int nextSizeExpected() {
+    static size_t nextSizeExpected() {
         return 0;
     }
 
@@ -58,7 +58,7 @@ public:
     }
 
     void checkTypeExpected(Type) {}
-    void checkFixedSizeExpected(int) {}
+    void checkFixedSizeExpected(size_t) {}
 };
 
 /// This class is used by both the ValidatingSerializer and ValidationParser
@@ -71,7 +71,7 @@ class AVRO_DECL Validator : private boost::noncopyable {
 public:
     explicit Validator(ValidSchema schema);
 
-    void setCount(int64_t val);
+    void setCount(size_t val);
 
     bool typeIsExpected(Type type) const {
         return (expectedTypesFlag_ & typeToFlag(type)) != 0;
@@ -81,7 +81,7 @@ public:
         return nextType_;
     }
 
-    int nextSizeExpected() const;
+    size_t nextSizeExpected() const;
 
     bool getCurrentRecordName(std::string &name) const;
     bool getNextFieldName(std::string &name) const;
@@ -93,7 +93,7 @@ public:
         advance();
     }
 
-    void checkFixedSizeExpected(int size) {
+    void checkFixedSizeExpected(size_t size) {
         if (nextSizeExpected() != size) {
             throw Exception("Wrong size for fixed, got {}, expected {}", size, nextSizeExpected());
         }
@@ -129,7 +129,7 @@ private:
     flag_t expectedTypesFlag_;
     bool compoundStarted_;
     bool waitingForCount_;
-    int64_t count_;
+    size_t count_;
 
     struct CompoundType {
         explicit CompoundType(NodePtr n) : node(std::move(n)), pos(0) {}

lang/c++/api/buffer/Buffer.hh

@@ -276,15 +276,15 @@ public:
      * Returns the number of chunks that contain free space.
      **/
 
-    int numChunks() const {
+    size_t numChunks() const {
         return pimpl_->numFreeChunks();
     }
 
     /**
      * Returns the number of chunks that contain data
      **/
 
-    int numDataChunks() const {
+    size_t numDataChunks() const {
         return pimpl_->numDataChunks();
     }
 
@@ -384,7 +384,7 @@ public:
      * Returns the number of chunks containing data.
      **/
 
-    int numChunks() const {
+    size_t numChunks() const {
         return pimpl_->numDataChunks();
     }
 
@@ -476,10 +476,10 @@ inline InputBuffer OutputBuffer::extractData(size_type bytes) {
 
 template<class BufferType>
 inline void toIovec(BufferType &buf, std::vector<struct iovec> &iov) {
-    const int chunks = buf.numChunks();
+    const size_t chunks = buf.numChunks();
     iov.resize(chunks);
     typename BufferType::const_iterator iter = buf.begin();
-    for (int i = 0; i < chunks; ++i) {
+    for (size_t i = 0; i < chunks; ++i) {
         iov[i].iov_base = const_cast<typename BufferType::data_type *>(iter->data());
         iov[i].iov_len = iter->size();
         ++iter;

lang/c++/api/buffer/BufferPrint.hh

@@ -47,7 +47,7 @@ hexPrint(std::ostream &os, BufferReader &reader) {
     std::ios_base::fmtflags savedFlags = os.flags();
 
     char sixteenBytes[16];
-    int offset = 0;
+    size_t offset = 0;
 
     os << std::setfill('0');
     os << std::hex;

lang/c++/api/buffer/BufferStreambuf.hh

@@ -135,7 +135,11 @@ protected:
                 memcpy(c, gptr(), toCopy);
                 c += toCopy;
                 bytesCopied += toCopy;
-                gbump(toCopy);
+                while (toCopy > std::numeric_limits<int>::max()) {
+                    gbump(std::numeric_limits<int>::max());
+                    toCopy -= std::numeric_limits<int>::max();
+                }
+                gbump(static_cast<int>(toCopy));
             }
 
             if (bytesCopied < len) {

lang/c++/api/buffer/detail/BufferDetail.hh

@@ -481,13 +481,13 @@ public:
     }
 
     /// The number of chunks containing data.  Used for debugging.
-    int numDataChunks() const {
+    size_t numDataChunks() const {
         return readChunks_.size();
     }
 
     /// The number of chunks containing free space (note that an entire chunk
     /// may not be free).  Used for debugging.
-    int numFreeChunks() const {
+    size_t numFreeChunks() const {
         return writeChunks_.size();
     }
 

lang/c++/impl/Compiler.cc

@@ -345,9 +345,10 @@ static LogicalType makeLogicalType(const Entity &e, const Object &m) {
     if (typeField == "decimal") {
         LogicalType decimalType(LogicalType::DECIMAL);
         try {
-            decimalType.setPrecision(getLongField(e, m, "precision"));
+            // Precision probably won't go over 38 and scale beyond -77/+77
+            decimalType.setPrecision(static_cast<int32_t>(getLongField(e, m, "precision")));
             if (containsField(m, "scale")) {
-                decimalType.setScale(getLongField(e, m, "scale"));
+                decimalType.setScale(static_cast<int32_t>(getLongField(e, m, "scale")));
             }
         } catch (Exception &ex) {
             // If any part of the logical type is malformed, per the standard we
@@ -395,12 +396,12 @@ static NodePtr makeEnumNode(const Entity &e,
 
 static NodePtr makeFixedNode(const Entity &e,
                              const Name &name, const Object &m) {
-    int v = static_cast<int>(getLongField(e, m, "size"));
+    int64_t v = getLongField(e, m, "size");
     if (v <= 0) {
         throw Exception("Size for fixed is not positive: {}", e.toString());
     }
     NodePtr node =
-        NodePtr(new NodeFixed(asSingleAttribute(name), asSingleAttribute(v)));
+        NodePtr(new NodeFixed(asSingleAttribute(name), asSingleAttribute(static_cast<size_t>(v))));
     if (containsField(m, "doc")) {
         node->setDoc(getDocField(e, m));
     }

lang/c++/impl/DataFile.cc

@@ -195,10 +195,10 @@ void DataFileWriterBase::sync() {
             os.push(boost::iostreams::back_inserter(temp));
             boost::iostreams::write(os, compressed.c_str(), compressed_size);
         }
-        temp.push_back((checksum >> 24) & 0xFF);
-        temp.push_back((checksum >> 16) & 0xFF);
-        temp.push_back((checksum >> 8) & 0xFF);
-        temp.push_back(checksum & 0xFF);
+        temp.push_back(static_cast<char>((checksum >> 24) & 0xFF));
+        temp.push_back(static_cast<char>((checksum >> 16) & 0xFF));
+        temp.push_back(static_cast<char>((checksum >> 8) & 0xFF));
+        temp.push_back(static_cast<char>(checksum & 0xFF));
         std::unique_ptr<InputStream> in = memoryInputStream(
             reinterpret_cast<const uint8_t *>(temp.data()), temp.size());
         int64_t byteCount = temp.size();
@@ -455,7 +455,7 @@ static ValidSchema makeSchema(const vector<uint8_t> &v) {
     istringstream iss(toString(v));
     ValidSchema vs;
     compileJsonSchema(iss, vs);
-    return ValidSchema(vs);
+    return vs;
 }
 
 void DataFileReaderBase::readHeader() {
@@ -527,8 +527,7 @@ void DataFileReaderBase::sync(int64_t position) {
             eof_ = true;
             return;
         }
-        int len =
-            std::min(static_cast<size_t>(SyncSize - i), n);
+        size_t len = std::min(SyncSize - i, n);
         memcpy(&sync_buffer[i], p, len);
         p += len;
         n -= len;

lang/c++/impl/FileStream.cc

@@ -94,7 +94,7 @@ struct FileBufferCopyIn : public BufferCopyIn {
     }
 
     bool read(uint8_t *b, size_t toRead, size_t &actual) final {
-        int n = ::read(fd_, b, toRead);
+        ssize_t n = ::read(fd_, b, toRead);
         if (n > 0) {
             actual = n;
             return true;

lang/c++/impl/LogicalType.cc

@@ -28,7 +28,7 @@ LogicalType::Type LogicalType::type() const {
     return type_;
 }
 
-void LogicalType::setPrecision(int precision) {
+void LogicalType::setPrecision(int32_t precision) {
     if (type_ != DECIMAL) {
         throw Exception("Only logical type DECIMAL can have precision");
     }
@@ -38,7 +38,7 @@ void LogicalType::setPrecision(int precision) {
     precision_ = precision;
 }
 
-void LogicalType::setScale(int scale) {
+void LogicalType::setScale(int32_t scale) {
     if (type_ != DECIMAL) {
         throw Exception("Only logical type DECIMAL can have scale");
     }

lang/c++/impl/Node.cc

@@ -146,7 +146,7 @@ void Node::setLogicalType(LogicalType logicalType) {
             if (type_ == AVRO_FIXED) {
                 // Max precision that can be supported by the current size of
                 // the FIXED type.
-                long maxPrecision = floor(log10(2.0) * (8.0 * fixedSize() - 1));
+                int32_t maxPrecision = static_cast<int32_t>(floor(log10(2.0) * (8.0 * static_cast<double>(fixedSize()) - 1)));
                 if (logicalType.precision() > maxPrecision) {
                     throw Exception(
                         "DECIMAL precision {} is too large for the "

lang/c++/impl/NodeImpl.cc

@@ -71,7 +71,7 @@ string escape(const string &unescaped) {
 // Wrap an indentation in a struct for ostream operator<<
 struct indent {
     explicit indent(size_t depth) : d(depth) {}
-    int d;
+    size_t d;
 };
 
 /// ostream operator for indent
@@ -83,7 +83,7 @@ std::ostream &operator<<(std::ostream &os, indent x) {
     return os;
 }
 
-void printCustomAttributes(const CustomAttributes &customAttributes, int depth,
+void printCustomAttributes(const CustomAttributes &customAttributes, size_t depth,
                            std::ostream &os) {
     std::map<std::string, std::string>::const_iterator iter =
         customAttributes.attributes().begin();
@@ -531,9 +531,9 @@ void NodeEnum::printJson(std::ostream &os, size_t depth) const {
     printName(os, nameAttribute_.get(), depth);
     os << indent(depth) << "\"symbols\": [\n";
 
-    int names = leafNameAttributes_.size();
+    size_t names = leafNameAttributes_.size();
     ++depth;
-    for (int i = 0; i < names; ++i) {
+    for (size_t i = 0; i < names; ++i) {
         if (i > 0) {
             os << ",\n";
         }
@@ -577,9 +577,9 @@ NodeMap::NodeMap() : NodeImplMap(AVRO_MAP) {
 
 void NodeUnion::printJson(std::ostream &os, size_t depth) const {
     os << "[\n";
-    int fields = leafAttributes_.size();
+    size_t fields = leafAttributes_.size();
     ++depth;
-    for (int i = 0; i < fields; ++i) {
+    for (size_t i = 0; i < fields; ++i) {
         if (i > 0) {
             os << ",\n";
         }

lang/c++/impl/Resolver.cc

@@ -349,7 +349,7 @@ class UnionParser : public Resolver {
 
         *readerChoice = choiceMapping_[writerChoice];
         auto *setter = reinterpret_cast<GenericUnionSetter *>(address + setFuncOffset_);
-        auto *value = reinterpret_cast<uint8_t *>(address + offset_);
+        uint8_t *value = address + offset_;
         uint8_t *location = (*setter)(value, *readerChoice);
 
         resolvers_[writerChoice]->parse(reader, location);
@@ -397,7 +397,7 @@ class NonUnionToUnionParser : public Resolver {
         auto *choice = reinterpret_cast<int64_t *>(address + choiceOffset_);
         *choice = choice_;
         auto *setter = reinterpret_cast<GenericUnionSetter *>(address + setFuncOffset_);
-        auto *value = reinterpret_cast<uint8_t *>(address + offset_);
+        uint8_t *value = address + offset_;
         uint8_t *location = (*setter)(value, choice_);
 
         resolver_->parse(reader, location);
@@ -424,7 +424,7 @@ class FixedSkipper : public Resolver {
     }
 
 protected:
-    int size_;
+    size_t size_;
 };
 
 class FixedParser : public Resolver {
@@ -436,12 +436,12 @@ class FixedParser : public Resolver {
 
     void parse(Reader &reader, uint8_t *address) const final {
         DEBUG_OUT("Reading fixed");
-        auto *location = reinterpret_cast<uint8_t *>(address + offset_);
+        uint8_t *location = address + offset_;
         reader.readFixed(location, size_);
     }
 
 protected:
-    int size_;
+    size_t size_;
     size_t offset_;
 };