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.

Sign up for GitHub

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

Jump to bottom

Fix compilation errors with Clang v16+. Also includes fixes for compiling with -std=c++23. #498

Merged
merged 1 commit into from
May 1, 2024
Merged

Fix compilation errors with Clang v16+. Also includes fixes for compiling with -std=c++23. #498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion sparta/sparta/log/MessageSource.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ namespace sparta
//! Move constructor
LogObject(LogObject&& rhp) :
src_(rhp.src_),
s_(std::move(rhp.s_.str())) // May unfortunately involve a copy
s_(rhp.s_.str()) // May unfortunately involve a copy
{ }

//! \brief Not Copy-constructable
Expand Down
4 changes: 2 additions & 2 deletions sparta/sparta/pipeViewer/Outputter.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ namespace sparta::pipeViewer
if(!dat.stringVector[i].empty()){
// We check if we have seen this exact pair, field and value before or not.
if(const auto& [val, str] = std::tie(dat.valueVector[i].first, dat.stringVector[i]);
stringMap.try_emplace(std::make_tuple(dat.pairId, i, val), str).second) {
stringMap.emplace(std::piecewise_construct, std::forward_as_tuple(dat.pairId, i, val), std::forward_as_tuple(str)).second) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround for a Clang bug: llvm/llvm-project#61415

This is essentially what try_emplace does under the hood, so performance shouldn't be affected.

// We add this mapping into out String Map file which we will
// use when reading back from the database.
string_file_ << dat.pairId
Expand Down Expand Up @@ -238,7 +238,7 @@ namespace sparta::pipeViewer
if(!dat.stringVector[i].empty()){
// We check if we have seen this exact pair, field and value before or not.
if(const auto& [val, str] = std::tie(dat.valueVector[i].first, dat.stringVector[i]);
stringMap.try_emplace(std::make_tuple(dat.pairId, i, val), str).second) {
stringMap.emplace(std::piecewise_construct, std::forward_as_tuple(dat.pairId, i, val), std::forward_as_tuple(str)).second) {
// We add this mapping into out String Map file which we will
// use when reading back from the database.
string_file_ << dat.pairId
Expand Down
2 changes: 1 addition & 1 deletion sparta/sparta/resources/Buffer.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ namespace sparta
*/
struct DataPointer {
private:
typename std::aligned_storage<sizeof(value_type), alignof(value_type)>::type object_memory_;
alignas(value_type) std::byte object_memory_[sizeof(value_type)];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::aligned_storage was deprecated in C++23: https://stackoverflow.com/questions/71828288/why-is-stdaligned-storage-to-be-deprecated-in-c23-and-what-to-use-instead


public:
DataPointer() { }
Expand Down
6 changes: 2 additions & 4 deletions sparta/sparta/statistics/dispatch/archives/ArchiveNode.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ class ArchiveNode
ArchiveNode() = default;

//! Construct a named node
ArchiveNode(const std::string & name) :
name_(name)
{}
ArchiveNode(const std::string & name);

//! Archive trees are built with the help of the
//! ReportStatisticsHierTree class. That class is
Expand All @@ -61,7 +59,7 @@ class ArchiveNode
ArchiveNode(name)
{}

virtual ~ArchiveNode() {}
virtual ~ArchiveNode();

//! Return the name that this node was originally created with
const std::string & getName() const {
Expand Down
15 changes: 9 additions & 6 deletions sparta/sparta/utils/SpartaSharedPointerAllocator.hpp
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup!

Original file line number Diff line number Diff line change
Expand Up @@ -285,16 +285,20 @@ namespace sparta
friend SpartaSharedPointer<PtrT>
allocate_sparta_shared_pointer(SpartaSharedPointerAllocator<PtrT> &, Args&&...args);

template<typename T>
struct AlignedStorage
{
alignas(T) std::byte buf[sizeof(T)];
};

// Internal MemoryBlock
struct MemBlock : public BaseAllocator::MemBlockBase
{
using RefCountType = typename SpartaSharedPointer<PointerT>::RefCount;

using RefCountAlignedStorage =
typename std::aligned_storage<sizeof(RefCountType), alignof(RefCountType)>::type;
using RefCountAlignedStorage = AlignedStorage<RefCountType>;

using PointerTAlignedStorage =
typename std::aligned_storage<sizeof(PointerT), alignof(PointerT)>::type;
using PointerTAlignedStorage = AlignedStorage<PointerT>;

template<typename ...ObjArgs>
MemBlock(SpartaSharedPointerAllocator * alloc_in, ObjArgs&&...obj_args) :
Expand Down Expand Up @@ -323,8 +327,7 @@ namespace sparta
class MemBlockVector
{
// Align the storage for the MemBlock
using MemBlockAlignedStorage =
typename std::aligned_storage<sizeof(MemBlock), alignof(MemBlock)>::type;
using MemBlockAlignedStorage = AlignedStorage<MemBlock>;

std::vector<MemBlockAlignedStorage> data_;
std::size_t size_ = 0;
Expand Down
16 changes: 8 additions & 8 deletions sparta/sparta/utils/Utils.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,56 +27,56 @@
/*!
* \brief Custom literal for uint64
*/
constexpr inline uint64_t operator "" _u64(unsigned long long i) {
constexpr inline uint64_t operator ""_u64(unsigned long long i) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clang throws a compiler error if there is a space between the "" and the _

return i;
}

/*!
* \brief Custom literal for uint32
*/
constexpr inline uint32_t operator "" _u32(unsigned long long i) {
constexpr inline uint32_t operator ""_u32(unsigned long long i) {
return i;
}

/*!
* \brief Custom literal for uint16
*/
constexpr inline uint16_t operator "" _u16(unsigned long long i) {
constexpr inline uint16_t operator ""_u16(unsigned long long i) {
return i;
}

/*!
* \brief Custom literal for uint8
*/
constexpr inline uint8_t operator "" _u8(unsigned long long i) {
constexpr inline uint8_t operator ""_u8(unsigned long long i) {
return i;
}

/*!
* \brief Custom literal for int64
*/
constexpr inline int64_t operator "" _64(unsigned long long i) {
constexpr inline int64_t operator ""_64(unsigned long long i) {
return i;
}

/*!
* \brief Custom literal for int32
*/
constexpr inline int32_t operator "" _32(unsigned long long i) {
constexpr inline int32_t operator ""_32(unsigned long long i) {
return i;
}

/*!
* \brief Custom literal for int16
*/
constexpr inline int16_t operator "" _16(unsigned long long i) {
constexpr inline int16_t operator ""_16(unsigned long long i) {
return i;
}

/*!
* \brief Custom literal for int8
*/
constexpr inline int8_t operator "" _8(unsigned long long i) {
constexpr inline int8_t operator ""_8(unsigned long long i) {
return i;
}

Expand Down
9 changes: 9 additions & 0 deletions sparta/src/StatisticsArchives.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,15 @@ void RootArchiveNode::saveTo(const std::string & dir)
archive_controller_->saveTo(dir);
}

ArchiveNode::ArchiveNode(const std::string & name) :
name_(name)
{
}

ArchiveNode::~ArchiveNode()
{
}

Comment on lines +199 to +207
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving these to the .cpp file helps avoid some pitfalls when you use an incomplete type with a smart pointer.

//Lazily walk up to the top of the tree to get the root node
RootArchiveNode * ArchiveNode::getRoot()
{
Expand Down
Loading