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

Upgrade clang-format & clang-tidy. #541

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
8 changes: 7 additions & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ Checks: "bugprone-*,\
-cert-err34-c,\
-cert-err58-cpp,\
-cert-flp30-c,\
-cert-int09-c,\
-cert-msc30-c,\
-cert-msc32-c,\
-cert-msc50-cpp,\
Expand All @@ -207,6 +208,7 @@ Checks: "bugprone-*,\
clang-analyzer-*,\
-clang-analyzer-cplusplus.NewDelete,\
-clang-analyzer-cplusplus.NewDeleteLeaks,\
-clang-analyzer-optin.core.EnumCastOutOfRange,\
-clang-analyzer-optin.performance.Padding,\
-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,\
*googletest*,\
Expand Down Expand Up @@ -268,6 +270,7 @@ Checks: "bugprone-*,\
-modernize-pass-by-value,\
-modernize-redundant-void-arg,\
-modernize-return-braced-init-list,\
-modernize-type-traits,\
-modernize-use-auto,\
-modernize-use-bool-literals,\
-modernize-use-default-member-init,\
Expand All @@ -280,6 +283,7 @@ Checks: "bugprone-*,\
-modernize-use-trailing-return-type,\
-modernize-use-using,\
performance-*,\
-performance-enum-size,\
-performance-move-const-arg,\
-performance-move-constructor-init,\
-performance-noexcept-move-constructor,\
Expand All @@ -293,6 +297,7 @@ Checks: "bugprone-*,\
-readability-convert-member-functions-to-static,
-readability-delete-null-pointer,\
-readability-else-after-return,\
-readability-enum-initial-value,\
-readability-function-cognitive-complexity,\
-readability-function-size,\
-readability-implicit-bool-conversion,\
Expand Down Expand Up @@ -327,9 +332,10 @@ ExtraArgs: [
]
WarningsAsErrors: '*'
HeaderFilterRegex: '.*'
AnalyzeTemporaryDtors: false
FormatStyle: 'file'
CheckOptions:
- key: bugprone-unused-return-value.AllowCastToVoid
value: true
- key: readability-braces-around-statements.ShortStatementLines
value: '1'
- key: hicpp-braces-around-statements.ShortStatementLines
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/run_pr_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ jobs:

- name: setup-ubuntu-clang-format
run:
pip install clang-format==17.0.6
pip install clang-format==19.1.0

- name: run clang-format
run: |
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ find_package(PythonInterp 3.6 REQUIRED)
# which perform static analysis and style checking on source files.
# When updating the version here, also update that used in the merge request
# config
find_package(ClangTools 17 COMPONENTS clang-format clang-tidy)
find_package(ClangTools 19 COMPONENTS clang-format clang-tidy)
if(TARGET ClangTools::clang-tidy)
ca_option(CA_CLANG_TIDY_FLAGS STRING
"Semi-color separated list of clang-tidy flags" "")
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ To install the dependencies on Ubuntu, open the terminal and run:
To install the recommended packages, run:
```sh
$ sudo apt install -y ninja-build doxygen python3-pip
$ sudo pip3 install lit virtualenv cmakelint clang-format==17.0.6
$ sudo pip3 install lit virtualenv cmakelint clang-format==19.1.0
```

### Compiling oneAPI Construction Kit
Expand Down
2 changes: 1 addition & 1 deletion clik/external/hal_cpu/source/hal_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class cpu_hal_platform : public hal::hal_t {
cpu_hal_platform() {
hal_device_info = &cpu_hal::setup_cpu_hal_device_info();

constexpr static uint32_t implemented_api_version = 6;
static constexpr uint32_t implemented_api_version = 6;
static_assert(implemented_api_version == hal_t::api_version,
"Implemented API version for CPU HAL does not match hal.h");
hal_info.platform_name = hal_device_info->target_name;
Expand Down
2 changes: 1 addition & 1 deletion clik/include/clik_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
struct clik_buffer;

struct CLIK_API clik_ndrange {
constexpr static uint32_t max_dimensions = 3;
static constexpr uint32_t max_dimensions = 3;
// Origin of the work 'grid'. Used when the first item is not at (0, 0, 0).
uint64_t offset[max_dimensions];
// Global size, i.e. total number of work-items in each dimension.
Expand Down
2 changes: 1 addition & 1 deletion clik/include/clik_hal_version.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@

#include <stdint.h>

constexpr static uint32_t supported_hal_api_version = 6;
static constexpr uint32_t supported_hal_api_version = 6;

#endif // _CLIK_CLIK_HAL_VERSION_H
4 changes: 2 additions & 2 deletions cmake/FindClangTools.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ foreach(component ${ClangTools_FIND_COMPONENTS})

if(result EQUAL 0)
# Strip all non-version text from the output, we only care about the
# <major>.<minor> version components.
string(REGEX MATCH "[0-9]+\\.[0-9]+" version_string ${version_string})
# <major> version component.
string(REGEX MATCH "[0-9]+" version_string ${version_string})
if(version_string VERSION_EQUAL ClangTools_FIND_VERSION)
# Success, found the correct version of the component.
set(ClangTools_${component}_FOUND TRUE)
Expand Down
2 changes: 1 addition & 1 deletion doc/getting-started.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ Recommended for Ubuntu 20.04

$ sudo apt update
$ sudo apt install -y ninja-build doxygen python3-pip
$ sudo pip3 install lit virtualenv cmakelint clang-format==17.0.6
$ sudo pip3 install lit virtualenv cmakelint clang-format==19.1.0

.. tip::
For ease of use ``python3`` and ``pip3`` can be symlinked to ``python`` and
Expand Down
2 changes: 1 addition & 1 deletion examples/hals/hal_cpu_client/source/hal_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace hal {
class hal_cpu_socket_client : public hal::hal_socket_client {
public:
hal_cpu_socket_client() : hal_socket_client(0) {
constexpr static uint32_t implemented_api_version = 6;
static constexpr uint32_t implemented_api_version = 6;
static_assert(
implemented_api_version == hal_t::api_version,
"Implemented API version for hal_socket_client does not match hal.h");
Expand Down
2 changes: 1 addition & 1 deletion examples/hals/hal_refsi_tutorial/source/hal_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class refsi_tutorial_hal : public hal::hal_t {

refsi_tutorial_hal() {
const char *target_name = "RefSi M1 Tutorial";
constexpr static uint32_t implemented_api_version = 6;
static constexpr uint32_t implemented_api_version = 6;
static_assert(implemented_api_version == hal_t::api_version,
"Implemented API version for RefSi HAL does not match hal.h");
hal_info.platform_name = target_name;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class ELFProgram {

reg_t find_symbol(const char *name) const;

constexpr static uint64_t invalid_address = ~0ull;
static constexpr uint64_t invalid_address = ~0ull;

private:
bool read_header(MemoryDevice &src, unit_id_t unit);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ struct RefSiGDevice : public RefSiDevice {
refsi_result setupHartLocalWindow(unsigned index);
void pre_run_kernel(slim_sim_t &sim, reg_t entry_point_addr);

constexpr const static unsigned window_index_elf = 0;
constexpr const static unsigned window_index_harts = 1;
static constexpr const unsigned window_index_elf = 0;
static constexpr const unsigned window_index_harts = 1;
unsigned max_harts = REFSI_SIM_MAX_HARTS;
refsi_addr_t elf_mem_mapped_addr = 0;
refsi_addr_t harts_mem_mapped_addr = 0;
Expand Down
2 changes: 1 addition & 1 deletion examples/refsi/hal_refsi/source/hal_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class refsi_hal : public hal::hal_t {
}

refsi_hal() {
constexpr static uint32_t implemented_api_version = 6;
static constexpr uint32_t implemented_api_version = 6;
static_assert(implemented_api_version == hal_t::api_version,
"Implemented API version for RefSi HAL does not match hal.h");
hal_info.num_devices = 1;
Expand Down
2 changes: 1 addition & 1 deletion hal/hal_remote/include/hal_remote/hal_transmitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class hal_transmitter {
/// Receive `size` bytes of data into `data`
/// @return true if the receive succeeds
virtual bool receive(void *data, uint32_t size) = 0;
virtual ~hal_transmitter(){};
virtual ~hal_transmitter() {};

void enable_debug(bool debug_enabled) { debug = debug_enabled; }
bool debug_enabled() { return debug; }
Expand Down
4 changes: 2 additions & 2 deletions hal/include/hal.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ namespace hal {
/// a hal. it provides access to device memory, program loading, execution
/// and information queries.
struct hal_device_t {
hal_device_t(hal_device_info_t *device_info) : device_info(device_info){};
virtual ~hal_device_t(){};
hal_device_t(hal_device_info_t *device_info) : device_info(device_info) {};
virtual ~hal_device_t() {};

/// @brief Find a specific kernel function in a compiled program.
///
Expand Down
2 changes: 1 addition & 1 deletion modules/cargo/include/cargo/allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class mallocator {
/// @brief Free allocated memory.
///
/// @param pointer Pointer to allocated memory.
void free(value_type *pointer) { cargo::free(pointer); }
void free(value_type *pointer) { cargo::free(static_cast<void *>(pointer)); }

/// @brief Allocate and construct an object.
///
Expand Down
73 changes: 34 additions & 39 deletions modules/cargo/include/cargo/detail/expected.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,9 @@ namespace detail {

// Trait for checking if a type is a cargo::expected
template <class T>
struct is_expected_impl : std::false_type {};
static constexpr bool is_expected_v = false;
template <class T, class E>
struct is_expected_impl<expected<T, E>> : std::true_type {};
template <class T>
using is_expected = is_expected_impl<std::decay_t<T>>;
static constexpr bool is_expected_v<expected<T, E>> = true;

template <class T, class E, class U>
using expected_enable_forward_value =
Expand All @@ -69,16 +67,13 @@ using expected_enable_from_other =
!std::is_convertible_v<const expected<U, G> &, T> &&
!std::is_convertible_v<const expected<U, G> &&, T>>;

template <class T, class U>
using is_void_or = std::conditional_t<std::is_void_v<T>, std::true_type, U>;

template <class T>
using is_copy_constructible_or_void =
is_void_or<T, std::is_copy_constructible<T>>;
static constexpr bool is_copy_constructible_or_void_v =
std::is_void_v<T> || std::is_copy_constructible_v<T>;

template <class T>
using is_move_constructible_or_void =
is_void_or<T, std::is_move_constructible<T>>;
static constexpr bool is_move_constructible_or_void_v =
std::is_void_v<T> || std::is_move_constructible_v<T>;

struct no_init_t {};
static constexpr no_init_t no_init{};
Expand Down Expand Up @@ -342,19 +337,21 @@ struct expected_operations_base : expected_storage_base<T, E> {

template <class... Args>
void construct(Args &&...args) noexcept {
new (std::addressof(this->m_val)) T(std::forward<Args>(args)...);
new (static_cast<void *>(std::addressof(this->m_val)))
T(std::forward<Args>(args)...);
this->m_has_val = true;
}

template <class Rhs>
void construct_with(Rhs &&rhs) noexcept {
new (std::addressof(this->m_val)) T(std::forward<Rhs>(rhs).get());
new (static_cast<void *>(std::addressof(this->m_val)))
T(std::forward<Rhs>(rhs).get());
this->m_has_val = true;
}

template <class... Args>
void construct_error(Args &&...args) noexcept {
new (std::addressof(this->m_unexpect))
new (static_cast<void *>(std::addressof(this->m_unexpect)))
unexpected<E>(std::forward<Args>(args)...);
this->m_has_val = false;
}
Expand Down Expand Up @@ -429,7 +426,7 @@ struct expected_operations_base<void, E> : expected_storage_base<void, E> {

template <class... Args>
void construct_error(Args &&...args) noexcept {
new (std::addressof(this->m_unexpect))
new (static_cast<void *>(std::addressof(this->m_unexpect)))
unexpected<E>(std::forward<Args>(args)...);
this->m_has_val = false;
}
Expand Down Expand Up @@ -463,9 +460,9 @@ struct expected_operations_base<void, E> : expected_storage_base<void, E> {
// This class manages conditionally having a trivial copy constructor
// This specialization is for when T and E are trivially copy constructible
template <class T, class E,
bool =
is_void_or<T, std::is_trivially_copy_constructible<T>>::value &&
std::is_trivially_copy_constructible_v<E>>
bool = (std::is_void_v<T> ||
std::is_trivially_copy_constructible_v<T>) &&
std::is_trivially_copy_constructible_v<E>>
struct expected_copy_base : expected_operations_base<T, E> {
using expected_operations_base<T, E>::expected_operations_base;
};
Expand All @@ -492,9 +489,9 @@ struct expected_copy_base<T, E, false> : expected_operations_base<T, E> {

// This class manages conditionally having a trivial move constructor
template <class T, class E,
bool =
is_void_or<T, std::is_trivially_move_constructible<T>>::value &&
std::is_trivially_move_constructible_v<E>>
bool = (std::is_void_v<T> ||
std::is_trivially_move_constructible_v<T>) &&
std::is_trivially_move_constructible_v<E>>
struct expected_move_base : expected_copy_base<T, E> {
using expected_copy_base<T, E>::expected_copy_base;
};
Expand All @@ -519,15 +516,14 @@ struct expected_move_base<T, E, false> : expected_copy_base<T, E> {
};

// This class manages conditionally having a trivial copy assignment operator
template <
class T, class E,
bool =
is_void_or<T, conjunction<std::is_trivially_copy_assignable<T>,
std::is_trivially_copy_constructible<T>,
std::is_trivially_destructible<T>>>::value &&
std::is_trivially_copy_assignable_v<E> &&
std::is_trivially_copy_constructible_v<E> &&
std::is_trivially_destructible_v<E>>
template <class T, class E,
bool = (std::is_void_v<T> ||
(std::is_trivially_copy_assignable_v<T> &&
std::is_trivially_copy_constructible_v<T> &&
std::is_trivially_destructible_v<T>)) &&
std::is_trivially_copy_assignable_v<E> &&
std::is_trivially_copy_constructible_v<E> &&
std::is_trivially_destructible_v<E>>
struct expected_copy_assign_base : expected_move_base<T, E> {
using expected_move_base<T, E>::expected_move_base;
};
Expand All @@ -549,15 +545,14 @@ struct expected_copy_assign_base<T, E, false> : expected_move_base<T, E> {
};

// This class manages conditionally having a trivial move assignment operator
template <
class T, class E,
bool = is_void_or<
T, conjunction<std::is_trivially_destructible<T>,
std::is_trivially_move_constructible<T>,
std::is_trivially_move_assignable<T>>>::value &&
std::is_trivially_destructible_v<E> &&
std::is_trivially_move_constructible_v<E> &&
std::is_trivially_move_assignable_v<E>>
template <class T, class E,
bool = (std::is_void_v<T> ||
(std::is_trivially_destructible_v<T> &&
std::is_trivially_move_constructible_v<T> &&
std::is_trivially_move_assignable_v<T>)) &&
std::is_trivially_destructible_v<E> &&
std::is_trivially_move_constructible_v<E> &&
std::is_trivially_move_assignable_v<E>>
struct expected_move_assign_base : expected_copy_assign_base<T, E> {
using expected_copy_assign_base<T, E>::expected_copy_assign_base;
};
Expand Down
5 changes: 3 additions & 2 deletions modules/cargo/include/cargo/detail/optional.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ using enable_from_other =
template <class T, class U>
using enable_assign_forward = std::enable_if_t<
!std::is_same_v<optional<T>, std::decay_t<U>> &&
!conjunction<std::is_scalar<T>, std::is_same<T, std::decay_t<U>>>::value &&
!(std::is_scalar_v<T> && std::is_same_v<T, std::decay_t<U>>) &&
std::is_constructible_v<T, U> && std::is_assignable_v<T &, U>>;

template <class T, class U, class Other>
Expand Down Expand Up @@ -162,7 +162,8 @@ struct optional_operations_base : optional_storage_base<T> {

template <class... Args>
void construct(Args &&...args) {
new (std::addressof(this->m_value)) T(std::forward<Args>(args)...);
new (static_cast<void *>(std::addressof(this->m_value)))
T(std::forward<Args>(args)...);
this->m_has_value = true;
}

Expand Down
5 changes: 3 additions & 2 deletions modules/cargo/include/cargo/dynamic_array.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,9 @@ class dynamic_array final {
if (nullptr == Begin) {
return cargo::bad_alloc;
}
std::for_each(Begin, End,
[&](reference item) { new (&item) value_type(); });
std::for_each(Begin, End, [&](reference item) {
new (static_cast<void *>(std::addressof(item))) value_type();
});
return cargo::success;
}

Expand Down
4 changes: 2 additions & 2 deletions modules/cargo/include/cargo/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ class error_or {
!(sizeof...(Args) == 0 &&
std::is_same_v<error_or, std::remove_reference_t<First>>)>>
error_or(First &&first, Args &&...args) : HasError(false) {
new (&ValueStorage) value_storage_type(std::forward<First>(first),
std::forward<Args>(args)...);
new (static_cast<void *>(&ValueStorage)) value_storage_type(
std::forward<First>(first), std::forward<Args>(args)...);
}

/// @brief Copy constructor.
Expand Down
Loading
Loading