From 4b1318238b1b7544d1156e9525b85f654ba2dc2d Mon Sep 17 00:00:00 2001 From: Arsentiy Milchakov Date: Thu, 23 Mar 2017 14:50:19 +0300 Subject: [PATCH] review fixes --- base/base_tests/buffer_vector_test.cpp | 7 ++-- base/buffer_vector.hpp | 12 ++++--- coding/coding_tests/file_container_test.cpp | 4 +-- coding/file_writer.cpp | 1 - coding/file_writer.hpp | 8 ++--- coding/multilang_utf8_string.cpp | 10 +++--- coding/writer.hpp | 36 ++++++++++----------- drape_frontend/watch/point.h | 6 ++-- drape_frontend/watch/text_engine.cpp | 4 +-- editor/opening_hours_ui.cpp | 2 +- geometry/spline.cpp | 6 ++-- indexer/categories_index.cpp | 4 +-- indexer/displacement_manager.hpp | 2 +- indexer/feature_impl.cpp | 27 ++++------------ indexer/feature_loader.cpp | 2 +- indexer/index.hpp | 2 +- indexer/old/feature_loader_101.cpp | 2 +- indexer/succinct_trie_builder.hpp | 3 +- openlr/openlr_simple_decoder.cpp | 2 +- openlr/openlr_stat/openlr_stat.cpp | 6 ++-- routing/restrictions_serialization.hpp | 2 +- 21 files changed, 66 insertions(+), 82 deletions(-) diff --git a/base/base_tests/buffer_vector_test.cpp b/base/base_tests/buffer_vector_test.cpp index b63d8b08b5..6b7949d488 100644 --- a/base/base_tests/buffer_vector_test.cpp +++ b/base/base_tests/buffer_vector_test.cpp @@ -3,6 +3,7 @@ #include "base/buffer_vector.hpp" #include "base/string_utils.hpp" +#include "std/numeric.hpp" #include "std/unique_ptr.hpp" namespace @@ -152,8 +153,7 @@ UNIT_TEST(BufferVectorInsert) } std::vector dataToInsert(insertLength); - for (size_t i = 0; i < insertLength; ++i) - dataToInsert[i] = 'a' + static_cast(i); + std::iota(dataToInsert.begin(), dataToInsert.end(), 'a'); b.insert(b.begin() + insertPos, dataToInsert.begin(), dataToInsert.end()); v.insert(v.begin() + insertPos, dataToInsert.begin(), dataToInsert.end()); @@ -210,8 +210,7 @@ UNIT_TEST(BufferVectorAppend) } std::vector dataToInsert(insertLength); - for (size_t i = 0; i < insertLength; ++i) - dataToInsert[i] = 'a' + static_cast(i); + std::iota(dataToInsert.begin(), dataToInsert.end(), 'a'); b.append(dataToInsert.begin(), dataToInsert.end()); v.insert(v.end(), dataToInsert.begin(), dataToInsert.end()); diff --git a/base/buffer_vector.hpp b/base/buffer_vector.hpp index e3565e6a26..2da0a75347 100644 --- a/base/buffer_vector.hpp +++ b/base/buffer_vector.hpp @@ -1,5 +1,6 @@ #pragma once #include "base/assert.hpp" +#include "base/checked_cast.hpp" #include "base/stl_iterator.hpp" #include @@ -371,9 +372,8 @@ public: template void insert(const_iterator where, TIt beg, TIt end) { - ptrdiff_t const pos = where - data(); - ASSERT_GREATER_OR_EQUAL(pos, 0, ()); - ASSERT_LESS_OR_EQUAL(pos, static_cast(size()), ()); + size_t const pos = base::asserted_cast(where - data()); + ASSERT_LESS_OR_EQUAL(pos, size(), ()); if (IsDynamic()) { @@ -384,9 +384,11 @@ public: size_t const n = end - beg; if (m_size + n <= N) { - if (static_cast(pos) != m_size) - for (ptrdiff_t i = m_size - 1; i >= pos; --i) + if (pos != m_size) + { + for (size_t i = m_size - 1; i >= pos && i < m_size; --i) Swap(m_static[i], m_static[i + n]); + } m_size += n; T * writableWhere = &m_static[0] + pos; diff --git a/coding/coding_tests/file_container_test.cpp b/coding/coding_tests/file_container_test.cpp index 9278d80fb4..15f4cea75c 100644 --- a/coding/coding_tests/file_container_test.cpp +++ b/coding/coding_tests/file_container_test.cpp @@ -289,12 +289,12 @@ UNIT_TEST(FilesMappingContainer_Smoke) { FilesContainerW writer(fName); - for (uint32_t i = 0; i < ARRAY_SIZE(key); ++i) + for (size_t i = 0; i < ARRAY_SIZE(key); ++i) { FileWriter w = writer.GetWriter(key[i]); for (uint32_t j = 0; j < count; ++j) { - uint32_t v = j + i; + uint32_t v = j + static_cast(i); w.Write(&v, sizeof(v)); } } diff --git a/coding/file_writer.cpp b/coding/file_writer.cpp index 61dd6ea2ae..4b95804fd7 100644 --- a/coding/file_writer.cpp +++ b/coding/file_writer.cpp @@ -30,7 +30,6 @@ uint64_t FileWriter::Pos() const void FileWriter::Seek(uint64_t pos) { - ASSERT_GREATER_OR_EQUAL(pos, 0, ()); m_pFileData->Seek(pos); } diff --git a/coding/file_writer.hpp b/coding/file_writer.hpp index 04f7919269..5b3ade04fd 100644 --- a/coding/file_writer.hpp +++ b/coding/file_writer.hpp @@ -29,11 +29,11 @@ public: explicit FileWriter(string const & fileName, Op operation = OP_WRITE_TRUNCATE, bool bTruncOnClose = false); - ~FileWriter(); + ~FileWriter() override; - void Seek(uint64_t pos); - uint64_t Pos() const; - void Write(void const * p, size_t size); + void Seek(uint64_t pos) override; + uint64_t Pos() const override; + void Write(void const * p, size_t size) override; void WritePaddingByEnd(size_t factor); void WritePaddingByPos(size_t factor); diff --git a/coding/multilang_utf8_string.cpp b/coding/multilang_utf8_string.cpp index 76d7c3cfa0..12a5ca5a67 100644 --- a/coding/multilang_utf8_string.cpp +++ b/coding/multilang_utf8_string.cpp @@ -26,6 +26,9 @@ StringUtf8Multilang::Languages const g_languages = {{ {"default", "Native for ea {"gsw", "Schwiizertüütsch"}, {"et", "Eesti"}, {"ku", "Kurdish"}, {"mn", "Mongolian"}, {"mk", "Македонски"}, {"lv", "Latviešu"}, {"hi", "हिन्दी"} }}; + +static_assert(g_languages.size() == StringUtf8Multilang::kMaxSupportedLanguages, + "With current encoding we are limited to 64 languages max."); } // namespace int8_t constexpr StringUtf8Multilang::kUnsupportedLanguageCode; @@ -44,9 +47,6 @@ StringUtf8Multilang::Languages const & StringUtf8Multilang::GetSupportedLanguage // static int8_t StringUtf8Multilang::GetLangIndex(string const & lang) { - static_assert(g_languages.size() == kMaxSupportedLanguages, - "With current encoding we are limited to 64 languages max."); - for (size_t i = 0; i < g_languages.size(); ++i) if (lang == g_languages[i].m_code) return static_cast(i); @@ -56,14 +56,14 @@ int8_t StringUtf8Multilang::GetLangIndex(string const & lang) // static char const * StringUtf8Multilang::GetLangByCode(int8_t langCode) { - if (langCode < 0 || langCode > static_cast(g_languages.size()) - 1) + if (langCode < 0 || langCode >= static_cast(g_languages.size())) return ""; return g_languages[langCode].m_code; } // static char const * StringUtf8Multilang::GetLangNameByCode(int8_t langCode) { - if (langCode < 0 || langCode > static_cast(g_languages.size()) - 1) + if (langCode < 0 || langCode >= static_cast(g_languages.size())) return ""; return g_languages[langCode].m_name; } diff --git a/coding/writer.hpp b/coding/writer.hpp index c7177499cd..59f56a623e 100644 --- a/coding/writer.hpp +++ b/coding/writer.hpp @@ -1,6 +1,7 @@ #pragma once #include "base/assert.hpp" #include "base/base.hpp" +#include "base/checked_cast.hpp" #include "base/exception.hpp" #include "std/algorithm.hpp" #include "std/shared_ptr.hpp" @@ -9,7 +10,6 @@ #include "std/vector.hpp" // Generic Writer. Not thread-safe. -// When SubWriter is used, pos can negative, so int64_t is used to store pos. class Writer { public: @@ -30,24 +30,22 @@ template class MemWriter : public Writer { public: - inline MemWriter(ContainerT & data) : m_Data(data), m_Pos(0) + inline explicit MemWriter(ContainerT & data) : m_Data(data), m_Pos(0) { static_assert(sizeof(typename ContainerT::value_type) == 1, ""); } - inline void Seek(uint64_t pos) + inline void Seek(uint64_t pos) override { - ASSERT_EQUAL(pos, static_cast(pos), ()); - ASSERT_GREATER_OR_EQUAL(pos, 0, ()); - m_Pos = static_cast(pos); + m_Pos = base::asserted_cast(pos); } - inline uint64_t Pos() const + inline uint64_t Pos() const override { return m_Pos; } - inline void Write(void const * p, size_t size) + inline void Write(void const * p, size_t size) override { intptr_t freeSize = m_Data.size() - m_Pos; if (freeSize < 0) @@ -75,7 +73,7 @@ private: // Original writer should not be used when SubWriter is active! // In destructor, SubWriter calls Seek() of original writer to the end of what has been written. template -class SubWriter +class SubWriter : public Writer { public: inline explicit SubWriter(WriterT & writer) @@ -86,14 +84,14 @@ public: { } - ~SubWriter() + ~SubWriter() override { ASSERT_EQUAL(m_offset, GetOffset(), ()); if (m_pos != m_maxPos) Seek(m_maxPos); } - inline void Seek(uint64_t pos) + inline void Seek(uint64_t pos) override { ASSERT_EQUAL(m_offset, GetOffset(), ()); m_writer.Seek(GetOffset() + pos); @@ -102,13 +100,13 @@ public: m_maxPos = max(m_maxPos, m_pos); } - inline uint64_t Pos() const + inline uint64_t Pos() const override { ASSERT_EQUAL(m_offset, GetOffset(), ()); return m_pos; } - inline void Write(void const * p, size_t size) + inline void Write(void const * p, size_t size) override { ASSERT_EQUAL(m_offset, GetOffset(), ()); m_writer.Write(p, size); @@ -132,22 +130,22 @@ private: }; template -class WriterPtr +class WriterPtr : public Writer { public: - WriterPtr(WriterT * p = 0) : m_p(p) {} + explicit WriterPtr(WriterT * p = 0) : m_p(p) {} - void Seek(uint64_t pos) + void Seek(uint64_t pos) override { m_p->Seek(pos); } - uint64_t Pos() const + uint64_t Pos() const override { return m_p->Pos(); } - void Write(void const * p, size_t size) + void Write(void const * p, size_t size) override { m_p->Write(p, size); } @@ -162,7 +160,7 @@ template class WriterSink { public: - inline WriterSink(WriterT & writer) : m_writer(writer), m_pos(0) {} + inline explicit WriterSink(WriterT & writer) : m_writer(writer), m_pos(0) {} inline void Write(void const * p, size_t size) { diff --git a/drape_frontend/watch/point.h b/drape_frontend/watch/point.h index c6ceca3f14..c2ca849958 100644 --- a/drape_frontend/watch/point.h +++ b/drape_frontend/watch/point.h @@ -232,10 +232,10 @@ inline double length(Container const & v) } template -inline int segment(std::vector > const & v, double length, double * out_length = NULL) +inline size_t segment(std::vector > const & v, double length, double * out_length = NULL) { if (v.size() < 2) - return -1; + return v.size(); size_t segment_num = 0; double segment_length = ml::distance(v[segment_num], v[segment_num + 1]); @@ -247,7 +247,7 @@ inline int segment(std::vector > const & v, double length, double } if (out_length) *out_length = length; - return static_cast(segment_num); + return segment_num; } template diff --git a/drape_frontend/watch/text_engine.cpp b/drape_frontend/watch/text_engine.cpp index 3b3cf15efd..c6ea397534 100644 --- a/drape_frontend/watch/text_engine.cpp +++ b/drape_frontend/watch/text_engine.cpp @@ -134,9 +134,9 @@ void text::warp(std::vector const & path, ml::text_options const & void text::warp_text_line(text::string_range const & text_line, std::vector const & path, ml::point_d & shift, bool flip) { - int segment_num = ml::segment(path, shift.x, &shift.x); + auto segment_num = ml::segment(path, shift.x, &shift.x); - if (segment_num == -1) + if (path.empty() || segment_num >= path.size() - 1) return; double tt_angle = path.front().angle(path.back()); diff --git a/editor/opening_hours_ui.cpp b/editor/opening_hours_ui.cpp index c05cc547b3..39099aaef9 100644 --- a/editor/opening_hours_ui.cpp +++ b/editor/opening_hours_ui.cpp @@ -110,7 +110,7 @@ osmoh::Timespan GetLongetsOpenSpan(osmoh::Timespan const & openingTime, return openingTime; osmoh::Timespan longestSpan{openingTime.GetStart(), excludeTime.front().GetStart()}; - for (size_t i = 0; i < excludeTime.size() - 1; ++i) + for (size_t i = 0; i + 1 < excludeTime.size(); ++i) { osmoh::Timespan nextOpenSpan{excludeTime[i].GetEnd(), excludeTime[i + 1].GetStart()}; longestSpan = SpanLength(longestSpan) > SpanLength(nextOpenSpan) ? longestSpan : nextOpenSpan; diff --git a/geometry/spline.cpp b/geometry/spline.cpp index a84b2dc6a6..29e82aad7e 100644 --- a/geometry/spline.cpp +++ b/geometry/spline.cpp @@ -200,10 +200,8 @@ void Spline::iterator::AdvanceBackward(double step) m_dist = 0.0; return; } - else - { - --m_index; - } + + --m_index; m_dist += m_spl->m_length[m_index]; } diff --git a/indexer/categories_index.cpp b/indexer/categories_index.cpp index af1513f314..c67b8416cc 100644 --- a/indexer/categories_index.cpp +++ b/indexer/categories_index.cpp @@ -52,7 +52,7 @@ namespace indexer { void CategoriesIndex::AddCategoryByTypeAndLang(uint32_t type, int8_t lang) { - ASSERT(lang >= 1 && lang <= static_cast(CategoriesHolder::kLocaleMapping.size()), + ASSERT(lang >= 1 && static_cast(lang) <= CategoriesHolder::kLocaleMapping.size(), ("Invalid lang code:", lang)); m_catHolder->ForEachNameByType(type, [&](TCategory::Name const & name) { @@ -69,7 +69,7 @@ void CategoriesIndex::AddCategoryByTypeAllLangs(uint32_t type) void CategoriesIndex::AddAllCategoriesInLang(int8_t lang) { - ASSERT(lang >= 1 && lang <= static_cast(CategoriesHolder::kLocaleMapping.size()), + ASSERT(lang >= 1 && static_cast(lang) <= CategoriesHolder::kLocaleMapping.size(), ("Invalid lang code:", lang)); m_catHolder->ForEachTypeAndCategory([&](uint32_t type, TCategory const & cat) { diff --git a/indexer/displacement_manager.hpp b/indexer/displacement_manager.hpp index 396ca81753..6e7f43b19b 100644 --- a/indexer/displacement_manager.hpp +++ b/indexer/displacement_manager.hpp @@ -127,7 +127,7 @@ public: static auto const maximumIgnoredZoom = feature::GetDrawableScaleRange( classif().GetTypeByPath({"railway", "station", "subway"})).first; - if (maximumIgnoredZoom != -1 && scale <= maximumIgnoredZoom) + if (maximumIgnoredZoom < 0 || scale <= maximumIgnoredZoom) { AddNodeToSorter(node, static_cast(scale)); acceptedNodes.Add(node); diff --git a/indexer/feature_impl.cpp b/indexer/feature_impl.cpp index 04f4d18a7a..a9d54d1d49 100644 --- a/indexer/feature_impl.cpp +++ b/indexer/feature_impl.cpp @@ -27,27 +27,14 @@ bool IsNumber(strings::UniString const & s) bool IsStreetNumber(strings::UniString const & s) { - size_t count = s.size(); - if (count >= 2) + if (s.size() < 2) + return false; + + /// add different localities in future, if it's a problem. + for (auto const & streetEnding : {"st", "nd", "rd", "th"}) { - /// add different localities in future, if it's a problem. - string streetEndings [] = {"st", "nd", "rd", "th"}; - for (size_t i = 0; i < ARRAY_SIZE(streetEndings); ++i) - { - size_t start = count - streetEndings[i].size(); - bool flag = false; - for (size_t j = 0; j < streetEndings[i].size(); ++j) - { - if (streetEndings[i][j] != static_cast(s[start + j])) - { - flag = true; - break; - } - } - if (flag) - return false; - } - return true; + if (strings::EndsWith(strings::ToUtf8(s), streetEnding)) + return true; } return false; } diff --git a/indexer/feature_loader.cpp b/indexer/feature_loader.cpp index 5149640729..5493febc6c 100644 --- a/indexer/feature_loader.cpp +++ b/indexer/feature_loader.cpp @@ -214,7 +214,7 @@ uint32_t LoaderCurrent::ParseGeometry(int scale) ASSERT_LESS ( scaleIndex, m_Info.GetScalesCount(), () ); points.push_back(m_pF->m_points.front()); - for (size_t i = 1; i < count - 1; ++i) + for (size_t i = 1; i + 1 < count; ++i) { // check for point visibility in needed scaleIndex if (static_cast((m_ptsSimpMask >> (2 * (i - 1))) & 0x3) <= scaleIndex) diff --git a/indexer/index.hpp b/indexer/index.hpp index 26feef5128..93b22b7aa6 100644 --- a/indexer/index.hpp +++ b/indexer/index.hpp @@ -103,7 +103,7 @@ private: feature::DataHeader const & header = pValue->GetHeader(); // Prepare needed covering. - int const lastScale = header.GetLastScale(); + auto const lastScale = header.GetLastScale(); // In case of WorldCoasts we should pass correct scale in ForEachInIntervalAndScale. if (scale > lastScale) diff --git a/indexer/old/feature_loader_101.cpp b/indexer/old/feature_loader_101.cpp index 3cc54b07b6..51e6822912 100644 --- a/indexer/old/feature_loader_101.cpp +++ b/indexer/old/feature_loader_101.cpp @@ -374,7 +374,7 @@ uint32_t LoaderImpl::ParseGeometry(int scale) ASSERT_LESS ( scaleIndex, m_Info.GetScalesCount(), () ); points.push_back(m_pF->m_points.front()); - for (size_t i = 1; i < count - 1; ++i) + for (size_t i = 1; i + 1 < count; ++i) { // check for point visibility in needed scaleIndex if (static_cast((m_ptsSimpMask >> (2 * (i - 1))) & 0x3) <= scaleIndex) diff --git a/indexer/succinct_trie_builder.hpp b/indexer/succinct_trie_builder.hpp index 76a605b9e8..0a17b5c40f 100644 --- a/indexer/succinct_trie_builder.hpp +++ b/indexer/succinct_trie_builder.hpp @@ -8,6 +8,7 @@ #include "coding/writer.hpp" #include "base/buffer_vector.hpp" +#include "base/checked_cast.hpp" #include "base/scope_guard.hpp" #include "base/string_utils.hpp" @@ -166,7 +167,7 @@ void BuildSuccinctTrie(TWriter & writer, TIter const beg, TIter const end) if (!node->m_isFinal) continue; finalNodeIds.push_back(static_cast(i)); - offsetTable.push_back(static_cast(valueWriter.Pos())); + offsetTable.push_back(base::asserted_cast(valueWriter.Pos())); node->m_valueList.Dump(valueWriter); } diff --git a/openlr/openlr_simple_decoder.cpp b/openlr/openlr_simple_decoder.cpp index 2d90aa4712..560ff8e9dc 100644 --- a/openlr/openlr_simple_decoder.cpp +++ b/openlr/openlr_simple_decoder.cpp @@ -128,7 +128,7 @@ void OpenLRSimpleDecoder::Decode(string const & outputFilename, int const segmen my::EraseIf(segments, [&filter](LinearSegment const & segment) { return !filter.Matches(segment); }); - if (segmentsToHandle != kHandleAllSegments && segmentsToHandle > 0 && + if (segmentsToHandle != kHandleAllSegments && segmentsToHandle >= 0 && static_cast(segmentsToHandle) < segments.size()) segments.resize(segmentsToHandle); diff --git a/openlr/openlr_stat/openlr_stat.cpp b/openlr/openlr_stat/openlr_stat.cpp index 5823c5a492..f238f27210 100644 --- a/openlr/openlr_stat/openlr_stat.cpp +++ b/openlr/openlr_stat/openlr_stat.cpp @@ -108,14 +108,14 @@ int main(int argc, char * argv[]) classificator::Load(); - auto const num_threads = static_cast(FLAGS_num_threads); + auto const numThreads = static_cast(FLAGS_num_threads); - vector indexes(num_threads); + vector indexes(numThreads); LoadIndexes(FLAGS_mwms_path, indexes); OpenLRSimpleDecoder::SegmentsFilter filter(FLAGS_ids_path, FLAGS_multipoints_only); OpenLRSimpleDecoder decoder(FLAGS_input, indexes); - decoder.Decode(FLAGS_output, FLAGS_limit, filter, num_threads); + decoder.Decode(FLAGS_output, FLAGS_limit, filter, numThreads); return 0; } diff --git a/routing/restrictions_serialization.hpp b/routing/restrictions_serialization.hpp index 33b472ca99..c942a14de2 100644 --- a/routing/restrictions_serialization.hpp +++ b/routing/restrictions_serialization.hpp @@ -194,7 +194,7 @@ private: return false; } - uint32_t const delta = static_cast(biasedDelta - 1); + uint32_t const delta = base::asserted_cast(biasedDelta - 1); restriction.m_featureIds[i] = static_cast(bits::ZigZagDecode(delta) + restriction.m_featureIds[i - 1]); }