Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix clang tidy warnings #23

Merged
merged 27 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
5879577
Add CMake preset with clang-tidy
MiKom Jul 23, 2024
3c223a8
Silence some clang-tidy warnings
MiKom Jul 23, 2024
114ca4d
Fix typos in parameter names
MiKom Jul 23, 2024
25135cc
Use !::empty() instead of ::size() > 0 for vectors
MiKom Jul 23, 2024
1b82980
Fix [[nodiscard]] warnings coming from KDBindings
MiKom Jul 23, 2024
62c25d6
Fix readability-redundant-member-init warnings
MiKom Jul 23, 2024
e7db91d
Fix misc-const-correctness warnings
MiKom Jul 23, 2024
51c98ee
Fix misc-use-anonymous-namespace warning
MiKom Jul 23, 2024
0ecc79b
Fix readability-redundant-inline-specifier warning
MiKom Jul 23, 2024
57c2130
Fix concurrency-mt-unsafe warnings
MiKom Jul 23, 2024
656b5f8
Fix bugprone-narrowing-conversions warning
MiKom Jul 23, 2024
310bad0
Dont't run clang-tidy on fmt
MiKom Jul 23, 2024
1083366
Fix misc-header-include-cycle from mio
MiKom Jul 23, 2024
dfe51f1
Fix modernize-avoid-c-arrays warning
MiKom Jul 23, 2024
0a90990
Fix readability-convert-member-functions-to-static
MiKom Jul 23, 2024
fbe039e
Fortify ~Object against exceptions
MiKom Jul 23, 2024
3539436
Fix bugprone-easily-swappable-parameters warnings
MiKom Jul 23, 2024
b9ec493
Fix misc-unused-parameters warnings
MiKom Jul 23, 2024
6d0f5d3
Fix misc-use-anonymous-namespace warning
MiKom Jul 23, 2024
270b683
Bump KDBindings ref
MiKom Jul 23, 2024
818c648
Fix readability-simplify-boolean-expr
MiKom Jul 23, 2024
aaa7f62
Fix modernize-use-auto warning
MiKom Jul 23, 2024
78e03dc
Fix modernize-use-nullptr warning
MiKom Jul 23, 2024
5a41b7b
Fix readability-make-member-function-const warning
MiKom Jul 23, 2024
7bacedc
Fix readability-inconsistent-declaration-parameter-name
MiKom Jul 23, 2024
5468592
Use std::array in linux_xcb_platform_window.cpp
MiKom Jul 24, 2024
cd9cdbb
Remove unnecessary semicolon
MiKom Jul 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CMakePresets.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,17 @@
"CMAKE_BUILD_TYPE": "Release"
}
},
{
"name": "release-clangtidy",
"displayName": "Release with clang-tidy",
"inherits": [
"release"
],
"binaryDir": "${sourceDir}/build/Release-clangtidy",
"cacheVariables": {
"CMAKE_CXX_CLANG_TIDY": "clang-tidy"
}
},
{
"name": "profile",
"displayName": "Profile",
Expand Down
4 changes: 3 additions & 1 deletion cmake/dependencies.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ if(NOT TARGET spdlog::spdlog)
GIT_TAG e69e5f977d458f2650bb346dadf2ad30c5320281 # 10.2.1
)
FetchContent_MakeAvailable(fmt)
set_target_properties(fmt PROPERTIES CXX_CLANG_TIDY "")
endif()
set(SPDLOG_FMT_EXTERNAL_HO ON)
# with this spdlog is included as a system library and won't e.g. trigger
Expand Down Expand Up @@ -73,7 +74,8 @@ if(NOT TARGET KDAB::KDBindings)
fetchcontent_declare(
KDBindings
GIT_REPOSITORY https://github.com/KDAB/KDBindings.git
GIT_TAG 4c4d36489b60a3ae5ce17a3dc0b6b7cbf43a9afa # v1.1.0-beta.2
GIT_TAG aab1cfd289296972ef32124f749e3b565b634535 # main as of 2024-07-17, needed for noexcept
# in destructors
USES_TERMINAL_DOWNLOAD YES USES_TERMINAL_UPDATE YES
)
fetchcontent_makeavailable(KDBindings)
Expand Down
4 changes: 3 additions & 1 deletion examples/gui_window/gui_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include <KDGui/window.h>
#include <KDGui/gui_events.h>

#include <tuple>

class ExampleWindow : public KDGui::Window
{
void mouseMoveEvent(KDGui::MouseMoveEvent *ev) override
Expand Down Expand Up @@ -68,7 +70,7 @@ int main()
w.title = "KDGui window example";
w.visible = true;

w.visible.valueChanged().connect([&app](bool visible) {
std::ignore = w.visible.valueChanged().connect([&app](bool visible) {
if (!visible) {
app.quit();
}
Expand Down
5 changes: 2 additions & 3 deletions src/KDFoundation/core_application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ using namespace KDFoundation;
CoreApplication *CoreApplication::ms_application = nullptr;

CoreApplication::CoreApplication(std::unique_ptr<AbstractPlatformIntegration> &&platformIntegration)
: Object()
, m_defaultLogger{ KDUtils::Logger::logger("default_log", spdlog::level::info) }
: m_defaultLogger{ KDUtils::Logger::logger("default_log", spdlog::level::info) }
, m_platformIntegration{ std::move(platformIntegration) }
, m_logger{ KDUtils::Logger::logger("core_application") }
{
Expand All @@ -39,7 +38,7 @@ CoreApplication::CoreApplication(std::unique_ptr<AbstractPlatformIntegration> &&
spdlog::set_default_logger(m_defaultLogger);

// Helps with debugging setup on remote hosts
if (const char *display = std::getenv("DISPLAY"))
if (const char *display = std::getenv("DISPLAY")) // NOLINT(concurrency-mt-unsafe)
SPDLOG_LOGGER_INFO(m_logger, "DISPLAY={}", display);

// Create a default postman object
Expand Down
3 changes: 1 addition & 2 deletions src/KDFoundation/file_descriptor_notifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@
using namespace KDFoundation;

FileDescriptorNotifier::FileDescriptorNotifier(int fd, NotificationType type)
: Object()
, m_fd{ fd }
: m_fd{ fd }
, m_type{ type }
{
assert(m_fd >= 0);
Expand Down
16 changes: 14 additions & 2 deletions src/KDFoundation/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,23 @@ void Object::deleteLater()

Object::~Object()
{
destroyed.emit(this);
try {
destroyed.emit(this);
} catch (...) {
SPDLOG_ERROR("Exception caught in ~Object({}) during Object::destroyed emission. Ignoring.",
objectName());
}

// Destroy the children in LIFO to be more like the stack
while (!m_children.empty()) {
childRemoved.emit(this, m_children.back().get());
Object *child = m_children.back().get();
try {
childRemoved.emit(this, child);
} catch (...) {
SPDLOG_ERROR("Exception thrown in ~Object on Object::childRemoved ({}) for child object {}. Ignoring.",
objectName(),
child->objectName());
}
m_children.pop_back();
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/KDFoundation/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ class KDFOUNDATION_API Object : public EventReceiver
{
public:
Object();
/// @warning This destructor emits #destroyed signal and #childRemoved for each child. If any of
/// the slots connected to these signals throws (which is UB in KDBindings anyway) the exception
/// will be eaten and error will be logged.
virtual ~Object();

// Not copyable
Expand Down
17 changes: 12 additions & 5 deletions src/KDFoundation/platform/linux/linux_platform_event_loop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
Contact KDAB at <info@kdab.com> for commercial licensing options.
*/

#if !defined(_GNU_SOURCE)
#error "Project must be compiled with _GNU_SOURCE define for GNU variant of strerror_r"
#endif

#include <KDFoundation/platform/linux/linux_platform_event_loop.h>
#include <KDFoundation/platform/linux/linux_platform_timer.h>
#include <KDFoundation/event.h>
Expand All @@ -17,10 +21,12 @@
#include <unistd.h>
#include <sys/eventfd.h>

#include <array>
#include <cstring>

using namespace KDFoundation;

LinuxPlatformEventLoop::LinuxPlatformEventLoop()
: AbstractPlatformEventLoop()
{
// We use epoll to multiplex as it has much better performance than
// select or poll.
Expand Down Expand Up @@ -64,9 +70,9 @@ LinuxPlatformEventLoop::~LinuxPlatformEventLoop()
void LinuxPlatformEventLoop::waitForEventsImpl(int timeout)
{
const int maxEventCount = 16;
epoll_event events[maxEventCount];
std::array<epoll_event, maxEventCount> events;

int eventCount = epoll_wait(m_epollHandle, events, maxEventCount, timeout);
const int eventCount = epoll_wait(m_epollHandle, events.data(), events.size(), timeout);
SPDLOG_DEBUG("epoll_wait() returned {} events within {} msecs", eventCount, timeout);

// Let interested parties know if something happened.
Expand Down Expand Up @@ -107,7 +113,7 @@ void LinuxPlatformEventLoop::waitForEventsImpl(int timeout)

void LinuxPlatformEventLoop::wakeUp()
{
eventfd_t value{ 1 };
const eventfd_t value{ 1 };
eventfd_write(m_eventfd, value);
}

Expand Down Expand Up @@ -175,7 +181,8 @@ bool LinuxPlatformEventLoop::unregisterFileDescriptor(int fd, FileDescriptorNoti
// proceed as if unregistering the file descriptor was successful.
// This way we ensure notifiers are reset / deleted properly.
if (rv && (errno != EBADF)) {
SPDLOG_ERROR("Failed to unregister file descriptor {}. Error {}: {}.", fd, errno, strerror(errno));
std::array<char, 64> buf;
SPDLOG_ERROR("Failed to unregister file descriptor {}. Error {}: {}.", fd, errno, strerror_r(errno, buf.data(), buf.size()));
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class KDFOUNDATION_API LinuxPlatformEventLoop : public AbstractPlatformEventLoop

int epollEventFromFdPlusType(int fd, FileDescriptorNotifier::NotificationType type);
int epollEventFromFdMinusType(int fd, FileDescriptorNotifier::NotificationType type);
int epollEventFromNotifierTypes(bool read, bool write, bool exception);
static int epollEventFromNotifierTypes(bool read, bool write, bool exception);

protected:
void waitForEventsImpl(int timeout) override;
Expand Down
26 changes: 14 additions & 12 deletions src/KDFoundation/platform/linux/linux_platform_timer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,33 +11,35 @@

#include <KDFoundation/platform/linux/linux_platform_timer.h>

#include <unistd.h>
#include <sys/timerfd.h>

#include "KDFoundation/core_application.h"
#include "KDFoundation/timer.h"
#include "KDFoundation/platform/linux/linux_platform_event_loop.h"

#include <unistd.h>
#include <sys/timerfd.h>

#include <array>
#include <tuple>

using namespace KDFoundation;

LinuxPlatformTimer::LinuxPlatformTimer(Timer *timer)
: m_notifier(timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC), FileDescriptorNotifier::NotificationType::Read)
{
m_notifier.triggered.connect([this, timer]() {
char buf[8];
const auto bytes = read(m_notifier.fileDescriptor(), buf, 8);
KD_UNUSED(bytes);
m_notifierConnection = m_notifier.triggered.connect([this, timer]() {
std::array<char, 8> buf;
std::ignore = read(m_notifier.fileDescriptor(), buf.data(), buf.size());
timer->timeout.emit();
});

timer->running.valueChanged().connect([this, timer](bool running) {
m_timerRunningConnection = timer->running.valueChanged().connect([this, timer](bool running) {
if (running) {
arm(timer->interval.get());
} else {
disarm();
}
});
timer->interval.valueChanged().connect([this, timer]() {
m_timerIntervalConnection = timer->interval.valueChanged().connect([this, timer]() {
if (timer->running.get()) {
arm(timer->interval.get());
}
Expand All @@ -61,7 +63,7 @@ void LinuxPlatformTimer::arm(std::chrono::microseconds us)
timespec time;
time.tv_sec = us.count() / 1'000'000;
time.tv_nsec = (us.count() - time.tv_sec * 1'000'000) * 1000;
itimerspec spec = {
const itimerspec spec = {
.it_interval = time,
.it_value = time
};
Expand All @@ -70,8 +72,8 @@ void LinuxPlatformTimer::arm(std::chrono::microseconds us)

void LinuxPlatformTimer::disarm()
{
timespec time = { 0, 0 };
itimerspec spec = {
const timespec time = { 0, 0 };
const itimerspec spec = {
.it_interval = time,
.it_value = time
};
Expand Down
5 changes: 5 additions & 0 deletions src/KDFoundation/platform/linux/linux_platform_timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

#include <chrono>

#include <kdbindings/signal.h>

#include <KDFoundation/platform/abstract_platform_timer.h>
#include <KDFoundation/file_descriptor_notifier.h>

Expand All @@ -35,6 +37,9 @@ class KDFOUNDATION_API LinuxPlatformTimer : public AbstractPlatformTimer
int fd;
} m_fdCloser;
FileDescriptorNotifier m_notifier;
KDBindings::ScopedConnection m_notifierConnection;
KDBindings::ScopedConnection m_timerRunningConnection;
KDBindings::ScopedConnection m_timerIntervalConnection;
};

} // namespace KDFoundation
4 changes: 2 additions & 2 deletions src/KDFoundation/platform/win32/win32_platform_timer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ inline Win32PlatformEventLoop *eventLoop()
Win32PlatformTimer::Win32PlatformTimer(Timer *timer)
: m_timer(timer)
{
timer->running.valueChanged().connect([this, timer](bool running) {
m_timerRunningConnection = timer->running.valueChanged().connect([this, timer](bool running) {
if (running) {
arm(timer->interval.get());
} else {
disarm();
}
});
timer->interval.valueChanged().connect([this, timer]() {
m_timerIntervalConnection = timer->interval.valueChanged().connect([this, timer]() {
if (timer->running.get()) {
arm(timer->interval.get());
}
Expand Down
3 changes: 3 additions & 0 deletions src/KDFoundation/platform/win32/win32_platform_timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

#include <chrono>
#include <windows.h>
#include <kdbindings/signal.h>
#undef max

#include <KDFoundation/platform/abstract_platform_timer.h>
Expand All @@ -33,6 +34,8 @@ class KDFOUNDATION_API Win32PlatformTimer : public AbstractPlatformTimer
static void callback(HWND hwnd, UINT uMsg, UINT_PTR timerId, DWORD dwTime);

Timer *m_timer;
KDBindings::ScopedConnection m_timerRunningConnection;
KDBindings::ScopedConnection m_timerIntervalConnection;
uintptr_t m_id{ 0 };
};

Expand Down
2 changes: 1 addition & 1 deletion src/KDGui/abstract_platform_window.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ AbstractPlatformWindow::AbstractPlatformWindow(Window *window, AbstractPlatformW
, m_type(type)
{
assert(window);
window->title.valueChanged().connect(&AbstractPlatformWindow::setTitle, this);
m_titleChangedConnection = window->title.valueChanged().connect(&AbstractPlatformWindow::setTitle, this);
}

AbstractPlatformWindow::Type AbstractPlatformWindow::type() const
Expand Down
3 changes: 3 additions & 0 deletions src/KDGui/abstract_platform_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include <KDGui/kdgui_keys.h>
#include <KDGui/gui_events.h>

#include <kdbindings/signal.h>

#include <string>

namespace KDGui {
Expand Down Expand Up @@ -86,6 +88,7 @@ class KDGUI_API AbstractPlatformWindow

protected:
Window *m_window;
KDBindings::ScopedConnection m_titleChangedConnection;
AbstractPlatformWindow::Type m_type;
};

Expand Down
8 changes: 5 additions & 3 deletions src/KDGui/gui_application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@

using namespace KDGui;

namespace {
#if defined(PLATFORM_LINUX)
static std::unique_ptr<KDGui::AbstractGuiPlatformIntegration> createLinuxIntegration()
std::unique_ptr<KDGui::AbstractGuiPlatformIntegration> createLinuxIntegration()
{
bool prefersXcb = false;

// TODO(doc): document this behavior
if (const char *preferredPlatform = std::getenv("KDGUI_PLATFORM")) {
if (const char *preferredPlatform = std::getenv("KDGUI_PLATFORM")) { // NOLINT(concurrency-mt-unsafe)
prefersXcb = std::string_view{ preferredPlatform } == "xcb";
}

Expand All @@ -46,7 +47,7 @@ static std::unique_ptr<KDGui::AbstractGuiPlatformIntegration> createLinuxIntegra
}
#endif

static std::unique_ptr<AbstractGuiPlatformIntegration> createPlatformIntegration()
std::unique_ptr<AbstractGuiPlatformIntegration> createPlatformIntegration()
{
#if defined(ANDROID)
return std::make_unique<AndroidPlatformIntegration>();
Expand All @@ -59,6 +60,7 @@ static std::unique_ptr<AbstractGuiPlatformIntegration> createPlatformIntegration
#endif
return {};
}
} // namespace

GuiApplication::GuiApplication(std::unique_ptr<AbstractGuiPlatformIntegration> &&platformIntegration)
: CoreApplication(platformIntegration ? std::move(platformIntegration) : createPlatformIntegration())
Expand Down
6 changes: 5 additions & 1 deletion src/KDGui/platform/linux/wayland/linux_wayland_clipboard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,23 @@ LinuxWaylandClipboard::LinuxWaylandClipboard(KDGui::LinuxWaylandPlatformIntegrat
{
}

void LinuxWaylandClipboard::dataOffer(wl_data_device *data_device, wl_data_offer *offer)
// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
void LinuxWaylandClipboard::dataOffer(wl_data_device * /*data_device*/, wl_data_offer *offer)
{
m_dataOffer = offer;
}

// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
void LinuxWaylandClipboard::dragEnter(wl_data_device *wl_data_device, uint32_t serial, wl_surface *surface, wl_fixed_t x, wl_fixed_t y, wl_data_offer *id)
{
}

// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
void LinuxWaylandClipboard::dragLeave(wl_data_device *wl_data_device)
{
}

// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
void LinuxWaylandClipboard::dragMotion(wl_data_device *wl_data_device, uint32_t time, wl_fixed_t x, wl_fixed_t y)
{
}
Expand Down
Loading