Skip to content

Commit

Permalink
Add clang-tidy checks and improve memory usage
Browse files Browse the repository at this point in the history
  • Loading branch information
@ntadej
ntadej committed Nov 15, 2023
1 parent 42984a1 commit a8452a1
Show file tree
Hide file tree
Showing 48 changed files with 734 additions and 530 deletions.
38 changes: 38 additions & 0 deletions .clang-tidy
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
Checks: >
clang-diagnostic-*,
clang-analyzer-*,
bugprone-*,
cppcoreguidelines-*,
google-*,
llvm-*,
misc-*,
modernize-*,
performance-*,
portability-*,
readability-*,
-bugprone-easily-swappable-parameters,
-cppcoreguidelines-avoid-c-arrays,
-cppcoreguidelines-avoid-const-or-ref-data-members,
-cppcoreguidelines-avoid-non-const-global-variables,
-cppcoreguidelines-non-private-member-variables-in-classes,
-cppcoreguidelines-pro-type-reinterpret-cast,
-cppcoreguidelines-pro-type-static-cast-downcast,
-cppcoreguidelines-pro-type-vararg,
-cppcoreguidelines-special-member-functions,
-google-readability-todo,
-llvm-header-guard,
-llvm-include-order,
-misc-include-cleaner,
-misc-no-recursion,
-misc-non-private-member-variables-in-classes,
-modernize-avoid-c-arrays,
-modernize-use-trailing-return-type,
-readability-convert-member-functions-to-static,
-readability-function-cognitive-complexity,
-readability-identifier-length,
-readability-redundant-access-specifiers,
-readability-uppercase-literal-suffix
WarningsAsErrors: '*'
HeaderFilterRegex: '^((?!vendor).)*$'
...
5 changes: 4 additions & 1 deletion .github/workflows/macOS.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ jobs:
- name: Build QMapLibre (Qt 6)
if: matrix.qt_series == 6
env:
MLN_QT_WITH_CLANG_TIDY: ${{ matrix.compiler != 'default' }}
run: |
mkdir build && cd build
qt-cmake ../source/ \
Expand All @@ -143,7 +145,8 @@ jobs:
-DCMAKE_CXX_COMPILER_LAUNCHER="ccache" \
-DCMAKE_INSTALL_PREFIX="../install" \
-DCMAKE_OSX_DEPLOYMENT_TARGET="${DEPLOYMENT_TARGET}" \
-DCMAKE_OSX_ARCHITECTURES="${DEPLOYMENT_ARCH}"
-DCMAKE_OSX_ARCHITECTURES="${DEPLOYMENT_ARCH}" \
-DMLN_QT_WITH_CLANG_TIDY="${MLN_QT_WITH_CLANG_TIDY}"
ninja
ninja test
ninja install
Expand Down
19 changes: 17 additions & 2 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ set(MLN_QT_QML_PLUGIN declarative_locationplugin_maplibre)
include(cmake/helpers.cmake)

# Options
set(MLN_QT_WITH_LOCATION ON CACHE BOOL "Build QMapLibreLocation")
set(MLN_QT_WITH_WIDGETS ON CACHE BOOL "Build QMapLibreWidgets")
option(MLN_QT_WITH_LOCATION "Build QMapLibreLocation" ON)
option(MLN_QT_WITH_WIDGETS "Build QMapLibreWidgets" ON)
option(MLN_QT_WITH_CLANG_TIDY "Build QMapLibre with clang-tidy checks enabled" OFF)

# Find Qt
find_package(QT NAMES Qt6 Qt5 COMPONENTS Core REQUIRED)
Expand Down Expand Up @@ -63,6 +64,20 @@ if (MSVC)
endforeach()
endif()

# clang-tidy
if (MLN_QT_WITH_CLANG_TIDY)
find_program(CLANG_TIDY_COMMAND NAMES clang-tidy)
if(NOT CLANG_TIDY_COMMAND)
message(FATAL_ERROR "ENABLE_CLANG_TIDY is ON but clang-tidy is not found!")
else()
message(STATUS "Found clang-tidy at ${CLANG_TIDY_COMMAND}")
endif()
# TODO: there are options which are only available on GCC(e.g. -Werror=maybe-uninitialized),
# that's why we need to disable this `unknown-warning-option` here.
# We could check if current compiler supports particular flag before enabling it.
set(CLANG_TIDY_COMMAND "${CLANG_TIDY_COMMAND};--extra-arg=-Wno-error=unknown-warning-option")
endif()

# Platform-specifics
if(ANDROID)
message(STATUS "Building for ABI: ${ANDROID_ABI}")
Expand Down
5 changes: 5 additions & 0 deletions src/core/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,11 @@ if (APPLE)
)
endif()

# Development specifics
if (MLN_QT_WITH_CLANG_TIDY)
set_target_properties(Core PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}")
endif()

# Export and installation
install(
EXPORT ${MLN_QT_NAME}CoreTargets
Expand Down
98 changes: 58 additions & 40 deletions src/core/conversion_p.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@

#include <optional>

namespace mbgl {
namespace style {
namespace conversion {
namespace mbgl::style::conversion {

std::string convertColor(const QColor &color);

Expand Down Expand Up @@ -58,9 +56,9 @@ class ConversionTraits<QVariant> {

if (iter != map.constEnd()) {
return iter.value();
} else {
return {};
}

return {};
}

template <class Fn>
Expand All @@ -87,9 +85,9 @@ class ConversionTraits<QVariant> {
if (value.type() == QVariant::Bool) {
#endif
return value.toBool();
} else {
return {};
}

return {};
}

static std::optional<float> toNumber(const QVariant &value) {
Expand All @@ -99,9 +97,9 @@ class ConversionTraits<QVariant> {
if (value.type() == QVariant::Int || value.type() == QVariant::Double) {
#endif
return value.toFloat();
} else {
return {};
}

return {};
}

static std::optional<double> toDouble(const QVariant &value) {
Expand All @@ -111,82 +109,104 @@ class ConversionTraits<QVariant> {
if (value.type() == QVariant::Int || value.type() == QVariant::Double) {
#endif
return value.toDouble();
} else {
return {};
}

return {};
}

static std::optional<std::string> toString(const QVariant &value) {
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
if (value.typeId() == QMetaType::QString) {
return value.toString().toStdString();
} else if (value.typeId() == QMetaType::QColor) {
}

if (value.typeId() == QMetaType::QColor) {
return convertColor(value.value<QColor>());
} else {
return {};
}
#else
if (value.type() == QVariant::String) {
return value.toString().toStdString();
} else if (value.type() == QVariant::Color) {
}

if (value.type() == QVariant::Color) {
return convertColor(value.value<QColor>());
} else {
return {};
}
#endif
return {};
}

static std::optional<Value> toValue(const QVariant &value) {
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
if (value.typeId() == QMetaType::Bool) {
return {value.toBool()};
} else if (value.typeId() == QMetaType::QString) {
}

if (value.typeId() == QMetaType::QString) {
return {value.toString().toStdString()};
} else if (value.typeId() == QMetaType::QColor) {
}

if (value.typeId() == QMetaType::QColor) {
return {convertColor(value.value<QColor>())};
} else if (value.typeId() == QMetaType::Int) {
return {int64_t(value.toInt())};
} else if (QMetaType::canConvert(value.metaType(), QMetaType(QMetaType::Double))) {
}

if (value.typeId() == QMetaType::Int) {
return {static_cast<int64_t>(value.toInt())};
}

if (QMetaType::canConvert(value.metaType(), QMetaType(QMetaType::Double))) {
return {value.toDouble()};
} else {
return {};
}
#else
if (value.type() == QVariant::Bool) {
return {value.toBool()};
} else if (value.type() == QVariant::String) {
}

if (value.type() == QVariant::String) {
return {value.toString().toStdString()};
} else if (value.type() == QVariant::Color) {
}

if (value.type() == QVariant::Color) {
return {convertColor(value.value<QColor>())};
} else if (value.type() == QVariant::Int) {
return {int64_t(value.toInt())};
} else if (value.canConvert(QVariant::Double)) {
}

if (value.type() == QVariant::Int) {
return {static_cast<int64_t>(value.toInt())};
}

if (value.canConvert(QVariant::Double)) {
return {value.toDouble()};
} else {
return {};
}
#endif
return {};
}

static std::optional<GeoJSON> toGeoJSON(const QVariant &value, Error &error) {
if (value.typeName() == QStringLiteral("QMapLibre::Feature")) {
return GeoJSON{QMapLibre::GeoJSON::asFeature(value.value<QMapLibre::Feature>())};
} else if (value.userType() == qMetaTypeId<QVector<QMapLibre::Feature>>()) {
}

if (value.userType() == qMetaTypeId<QVector<QMapLibre::Feature>>()) {
return featureCollectionToGeoJSON(value.value<QVector<QMapLibre::Feature>>());
} else if (value.userType() == qMetaTypeId<QList<QMapLibre::Feature>>()) {
}

if (value.userType() == qMetaTypeId<QList<QMapLibre::Feature>>()) {
return featureCollectionToGeoJSON(value.value<QList<QMapLibre::Feature>>());
} else if (value.userType() == qMetaTypeId<std::list<QMapLibre::Feature>>()) {
}

if (value.userType() == qMetaTypeId<std::list<QMapLibre::Feature>>()) {
return featureCollectionToGeoJSON(value.value<std::list<QMapLibre::Feature>>());
}

#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
} else if (value.typeId() != QMetaType::QByteArray) {
if (value.typeId() != QMetaType::QByteArray) {
#else
} else if (value.type() != QVariant::ByteArray) {
if (value.type() != QVariant::ByteArray) {
#endif
error = {"JSON data must be in QByteArray"};
return {};
}

QByteArray data = value.toByteArray();
const QByteArray data = value.toByteArray();
return parseGeoJSON(std::string(data.constData(), data.size()), error);
}

Expand All @@ -212,6 +232,4 @@ inline std::string convertColor(const QColor &color) {
.toStdString();
}

} // namespace conversion
} // namespace style
} // namespace mbgl
} // namespace mbgl::style::conversion
29 changes: 16 additions & 13 deletions src/core/geojson.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ mbgl::Value asPropertyValue(const QVariant &value) {
std::unordered_map<std::string, mbgl::Value> mbglMap;
mbglMap.reserve(map.size());
for (auto it = map.constBegin(); it != map.constEnd(); ++it) {
mbglMap.emplace(std::make_pair(it.key().toStdString(), asPropertyValue(it.value())));
mbglMap.emplace(it.key().toStdString(), asPropertyValue(it.value()));
}
return mbglMap;
};
Expand All @@ -92,9 +92,9 @@ mbgl::Value asPropertyValue(const QVariant &value) {
case QMetaType::Bool:
return {value.toBool()};
case QMetaType::ULongLong:
return {uint64_t(value.toULongLong())};
return {static_cast<uint64_t>(value.toULongLong())};
case QMetaType::LongLong:
return {int64_t(value.toLongLong())};
return {static_cast<int64_t>(value.toLongLong())};
case QMetaType::Double:
return {value.toDouble()};
case QMetaType::QString:
Expand All @@ -118,9 +118,9 @@ mbgl::FeatureIdentifier asFeatureIdentifier(const QVariant &id) {
case QMetaType::UnknownType:
return {};
case QMetaType::ULongLong:
return {uint64_t(id.toULongLong())};
return {static_cast<uint64_t>(id.toULongLong())};
case QMetaType::LongLong:
return {int64_t(id.toLongLong())};
return {static_cast<int64_t>(id.toLongLong())};
case QMetaType::Double:
return {id.toDouble()};
case QMetaType::QString:
Expand All @@ -144,24 +144,27 @@ mbgl::GeoJSONFeature asFeature(const Feature &feature) {
const Coordinates &points = feature.geometry.first().first();
if (points.size() == 1) {
return {asPoint(points.first()), std::move(properties), std::move(id)};
} else {
return {asMultiPoint(points), std::move(properties), std::move(id)};
}
} else if (feature.type == Feature::LineStringType) {
return {asMultiPoint(points), std::move(properties), std::move(id)};
}

if (feature.type == Feature::LineStringType) {
const CoordinatesCollection &lineStrings = feature.geometry.first();
if (lineStrings.size() == 1) {
return {asLineString(lineStrings.first()), std::move(properties), std::move(id)};
} else {
return {asMultiLineString(lineStrings), std::move(properties), std::move(id)};
}
} else { // PolygonType
return {asMultiLineString(lineStrings), std::move(properties), std::move(id)};
}

if (feature.type == Feature::PolygonType) {
const CoordinatesCollections &polygons = feature.geometry;
if (polygons.size() == 1) {
return {asPolygon(polygons.first()), std::move(properties), std::move(id)};
} else {
return {asMultiPolygon(polygons), std::move(properties), std::move(id)};
}
return {asMultiPolygon(polygons), std::move(properties), std::move(id)};
}

throw std::runtime_error("Unsupported feature type");
};

} // namespace QMapLibre::GeoJSON
Loading

0 comments on commit a8452a1

Please sign in to comment.