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

Move to clang-format 15 #621

Merged
merged 6 commits into from
Jul 18, 2023
Merged
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
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -400,12 +400,14 @@ jobs:

# Job to run clang-format and report errors
format:
runs-on: ubuntu-20.04
runs-on: ubuntu-22.04
# We don't need to do the build for this job, but we need to configure it to get the clang-format target
steps:
- uses: actions/checkout@v3
- name: Install clang-tidy and clang-format
run: sudo apt install clang-tidy-9 clang-format-9
run: |
sudo apt update
sudo apt install clang-tidy-15 clang-format-15
- name: Configure CMake
run: cmake -B ${{github.workspace}}/build -DSNMALLOC_USE_CXX17=ON
# Run the clang-format check and error if it generates a diff
Expand All @@ -417,7 +419,7 @@ jobs:
git diff --exit-code
- name: Run clang-tidy
run: |
clang-tidy-9 src/snmalloc/override/malloc.cc -header-filter="`pwd`/*" -warnings-as-errors='*' -export-fixes=tidy.fail -- -std=c++17 -mcx16 -DSNMALLOC_PLATFORM_HAS_GETENTROPY=0
clang-tidy-15 src/snmalloc/override/malloc.cc -header-filter="`pwd`/*" -warnings-as-errors='*' -export-fixes=tidy.fail -- -std=c++17 -mcx16 -DSNMALLOC_PLATFORM_HAS_GETENTROPY=0
if [ -f tidy.fail ] ; then
cat tidy.fail
exit 1
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ function(clangformat_targets)
# tool. It does not work with older versions as AfterCaseLabel is not supported
# in earlier versions.
find_program(CLANG_FORMAT NAMES
clang-format90 clang-format-9)
clang-format150 clang-format-15)

# If we've found a clang-format tool, generate a target for it, otherwise emit
# a warning.
Expand Down
120 changes: 56 additions & 64 deletions src/snmalloc/aal/aal_concept.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,87 +14,79 @@ namespace snmalloc
* machine word size, and an upper bound on the address space size
*/
template<typename AAL>
concept IsAAL_static_members = requires()
{
typename std::integral_constant<uint64_t, AAL::aal_features>;
typename std::integral_constant<int, AAL::aal_name>;
typename std::integral_constant<std::size_t, AAL::bits>;
typename std::integral_constant<std::size_t, AAL::address_bits>;
};
concept IsAAL_static_members =
requires() {
typename std::integral_constant<uint64_t, AAL::aal_features>;
typename std::integral_constant<int, AAL::aal_name>;
typename std::integral_constant<std::size_t, AAL::bits>;
typename std::integral_constant<std::size_t, AAL::address_bits>;
};

/**
* AALs provide a prefetch operation.
*/
template<typename AAL>
concept IsAAL_prefetch = requires(void* ptr)
{
{
AAL::prefetch(ptr)
}
noexcept->ConceptSame<void>;
};
concept IsAAL_prefetch = requires(void* ptr) {
{
AAL::prefetch(ptr)
} noexcept -> ConceptSame<void>;
};

/**
* AALs provide a notion of high-precision timing.
*/
template<typename AAL>
concept IsAAL_tick = requires()
{
{
AAL::tick()
}
noexcept->ConceptSame<uint64_t>;
};
concept IsAAL_tick = requires() {
{
AAL::tick()
} noexcept -> ConceptSame<uint64_t>;
};

template<typename AAL>
concept IsAAL_capptr_methods =
requires(capptr::Chunk<void> auth, capptr::AllocFull<void> ret, size_t sz)
{
/**
* Produce a pointer with reduced authority from a more privilged pointer.
* The resulting pointer will have base at auth's address and length of
* exactly sz. auth+sz must not exceed auth's limit.
*/
{
AAL::template capptr_bound<void, capptr::bounds::Chunk>(auth, sz)
}
noexcept->ConceptSame<capptr::Chunk<void>>;
requires(capptr::Chunk<void> auth, capptr::AllocFull<void> ret, size_t sz) {
/**
* Produce a pointer with reduced authority from a more privilged pointer.
* The resulting pointer will have base at auth's address and length of
* exactly sz. auth+sz must not exceed auth's limit.
*/
{
AAL::template capptr_bound<void, capptr::bounds::Chunk>(auth, sz)
} noexcept -> ConceptSame<capptr::Chunk<void>>;

/**
* "Amplify" by copying the address of one pointer into one of higher
* privilege. The resulting pointer differs from auth only in address.
*/
{
AAL::capptr_rebound(auth, ret)
}
noexcept->ConceptSame<capptr::Chunk<void>>;
/**
* "Amplify" by copying the address of one pointer into one of higher
* privilege. The resulting pointer differs from auth only in address.
*/
{
AAL::capptr_rebound(auth, ret)
} noexcept -> ConceptSame<capptr::Chunk<void>>;

/**
* Round up an allocation size to a size this architecture can represent.
* While there may also, in general, be alignment requirements for
* representability, in snmalloc so far we have not had reason to consider
* these explicitly: when we use our...
*
* - sizeclass machinery (for user-facing data), we assume that all
* sizeclasses describe architecturally representable aligned-and-sized
* regions
*
* - Range machinery (for internal meta-data), we always choose NAPOT
* regions big enough for the requested size (returning space above the
* allocation within such regions for use as smaller NAPOT regions).
*
* That is, capptr_size_round is not needed on the user-facing fast paths,
* merely internally for bootstrap and metadata management.
*/
{
AAL::capptr_size_round(sz)
}
noexcept->ConceptSame<size_t>;
};
/**
* Round up an allocation size to a size this architecture can represent.
* While there may also, in general, be alignment requirements for
* representability, in snmalloc so far we have not had reason to consider
* these explicitly: when we use our...
*
* - sizeclass machinery (for user-facing data), we assume that all
* sizeclasses describe architecturally representable aligned-and-sized
* regions
*
* - Range machinery (for internal meta-data), we always choose NAPOT
* regions big enough for the requested size (returning space above the
* allocation within such regions for use as smaller NAPOT regions).
*
* That is, capptr_size_round is not needed on the user-facing fast paths,
* merely internally for bootstrap and metadata management.
*/
{
AAL::capptr_size_round(sz)
} noexcept -> ConceptSame<size_t>;
};

template<typename AAL>
concept IsAAL = IsAAL_static_members<AAL>&& IsAAL_prefetch<AAL>&&
IsAAL_tick<AAL>&& IsAAL_capptr_methods<AAL>;
concept IsAAL = IsAAL_static_members<AAL> && IsAAL_prefetch<AAL> &&
IsAAL_tick<AAL> && IsAAL_capptr_methods<AAL>;

} // namespace snmalloc
#endif
6 changes: 2 additions & 4 deletions src/snmalloc/backend/fixedglobalconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,11 @@ namespace snmalloc
* C++, and not just its initializer fragment, to initialize a non-prefix
* subset of the flags (in any order, at that).
*/
static constexpr Flags Options = []() constexpr
{
static constexpr Flags Options = []() constexpr {
Flags opts = {};
opts.HasDomesticate = true;
return opts;
}
();
}();

// This needs to be a forward reference as the
// thread local state will need to know about this.
Expand Down
7 changes: 4 additions & 3 deletions src/snmalloc/ds/aba.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,10 @@ namespace snmalloc
error("Only one inflight ABA operation at a time is allowed.");
operation_in_flight = true;
# endif
return Cmp{{independent.ptr.load(std::memory_order_relaxed),
independent.aba.load(std::memory_order_relaxed)},
this};
return Cmp{
{independent.ptr.load(std::memory_order_relaxed),
independent.aba.load(std::memory_order_relaxed)},
this};
}

struct Cmp
Expand Down
4 changes: 2 additions & 2 deletions src/snmalloc/ds_core/helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ namespace snmalloc
}
std::array<char, 20> buf{{0}};
const char digits[] = "0123456789";
for (long i = long(buf.size() - 1); i >= 0; i--)
for (long i = static_cast<long>(buf.size() - 1); i >= 0; i--)
{
buf[static_cast<size_t>(i)] = digits[s % 10];
s /= 10;
Expand Down Expand Up @@ -356,7 +356,7 @@ namespace snmalloc
const char hexdigits[] = "0123456789abcdef";
// Length of string including null terminator
static_assert(sizeof(hexdigits) == 0x11);
for (long i = long(buf.size() - 1); i >= 0; i--)
for (long i = static_cast<long>(buf.size() - 1); i >= 0; i--)
{
buf[static_cast<size_t>(i)] = hexdigits[s & 0xf];
s >>= 4;
Expand Down
8 changes: 4 additions & 4 deletions src/snmalloc/ds_core/mitigations.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,10 @@ namespace snmalloc
*/
full_checks + cheri_checks + clear_meta - freelist_forward_edge -
pal_enforce_access :
/**
* clear_meta is important on CHERI to avoid leaking capabilities.
*/
sanity_checks + cheri_checks + clear_meta;
/**
* clear_meta is important on CHERI to avoid leaking capabilities.
*/
sanity_checks + cheri_checks + clear_meta;
#else
CHECK_CLIENT ? full_checks : no_checks;
#endif
Expand Down
85 changes: 34 additions & 51 deletions src/snmalloc/ds_core/redblacktree.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ namespace snmalloc
* ID.
*/
template<typename Rep>
concept RBRepTypes = requires()
{
typename Rep::Handle;
typename Rep::Contents;
};
concept RBRepTypes = requires() {
typename Rep::Handle;
typename Rep::Contents;
};

/**
* The representation must define operations on the holder and contents
Expand All @@ -41,50 +40,36 @@ namespace snmalloc
*/
template<typename Rep>
concept RBRepMethods =
requires(typename Rep::Handle hp, typename Rep::Contents k, bool b)
{
{
Rep::get(hp)
}
->ConceptSame<typename Rep::Contents>;
{
Rep::set(hp, k)
}
->ConceptSame<void>;
{
Rep::is_red(k)
}
->ConceptSame<bool>;
{
Rep::set_red(k, b)
}
->ConceptSame<void>;
{
Rep::ref(b, k)
}
->ConceptSame<typename Rep::Handle>;
{
Rep::null
}
->ConceptSameModRef<const typename Rep::Contents>;
{
typename Rep::Handle
requires(typename Rep::Handle hp, typename Rep::Contents k, bool b) {
{
Rep::get(hp)
} -> ConceptSame<typename Rep::Contents>;
{
Rep::set(hp, k)
} -> ConceptSame<void>;
{
Rep::is_red(k)
} -> ConceptSame<bool>;
{
const_cast<
Rep::set_red(k, b)
} -> ConceptSame<void>;
{
Rep::ref(b, k)
} -> ConceptSame<typename Rep::Handle>;
{
Rep::null
} -> ConceptSameModRef<const typename Rep::Contents>;
{
typename Rep::Handle{const_cast<
std::remove_const_t<std::remove_reference_t<decltype(Rep::root)>>*>(
&Rep::root)
}
}
->ConceptSame<typename Rep::Handle>;
};
&Rep::root)}
} -> ConceptSame<typename Rep::Handle>;
};

template<typename Rep>
concept RBRep = //
RBRepTypes<Rep> //
&& RBRepMethods<Rep> //
&& ConceptSame<
decltype(Rep::null),
std::add_const_t<typename Rep::Contents>>;
RBRepTypes<Rep> && RBRepMethods<Rep> &&
ConceptSame<decltype(Rep::null), std::add_const_t<typename Rep::Contents>>;
#endif

/**
Expand Down Expand Up @@ -275,7 +260,7 @@ namespace snmalloc
std::array<RBStep, 128> path;
size_t length = 0;

RBPath(typename Rep::Handle root) : path{}
RBPath(typename Rep::Handle root)
{
path[0].set(root, false);
length = 1;
Expand Down Expand Up @@ -490,8 +475,7 @@ namespace snmalloc
*/
path.move(true);
while (path.move(false))
{
}
{}

K curr = path.curr();

Expand All @@ -510,8 +494,8 @@ namespace snmalloc
// If we had a left child, replace ourselves with the extracted value
// from above
Rep::set_red(curr, Rep::is_red(splice));
get_dir(true, curr) = K(get_dir(true, splice));
get_dir(false, curr) = K(get_dir(false, splice));
get_dir(true, curr) = K{get_dir(true, splice)};
get_dir(false, curr) = K{get_dir(false, splice)};
splice = curr;
path.fixup();
}
Expand Down Expand Up @@ -742,8 +726,7 @@ namespace snmalloc

auto path = get_root_path();
while (path.move(true))
{
}
{}

K result = path.curr();

Expand Down
3 changes: 2 additions & 1 deletion src/snmalloc/global/memcpy.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ namespace snmalloc
* It's not entirely clear what we would do if this were not the case.
* Best not think too hard about it now.
*/
static_assert(alignof(void*) == sizeof(void*));
static_assert(
alignof(void*) == sizeof(void*)); // NOLINT(misc-redundant-expression)

static constexpr size_t LargestRegisterSize = 16;

Expand Down
Loading