diff --git a/.vscode/launch.json b/.vscode/launch.json deleted file mode 100644 index c7ad6761dd..0000000000 --- a/.vscode/launch.json +++ /dev/null @@ -1,33 +0,0 @@ -{ - "version": "0.2.0", - "configurations": [ - { - "name": "(ctest) Launch", - "type": "cppdbg", - "cwd": "${cmake.testWorkingDirectory}", - "request": "launch", - "program": "${cmake.testProgram}", - "args": [ "${cmake.testArgs}" ], - // other options... - }, - { - "name": "Debug on Windows", - "type": "cppvsdbg", - "request": "launch", - "program": "${workspaceFolder}/build/", - "args": [], - "stopAtEntry": false, - "cwd": "${workspaceFolder}", - "environment": [], - "externalConsole": false - }, - { - "name": "Debug on Linux", - "type": "gdb", - "request": "launch", - "target": "${workspaceFolder}/bazel-bin/", - "cwd": "${workspaceRoot}", - "valuesFormatting": "parseText" - } - ] -} diff --git a/api/include/opentelemetry/context/context.h b/api/include/opentelemetry/context/context.h index 924036efad..a8c5fd2a91 100644 --- a/api/include/opentelemetry/context/context.h +++ b/api/include/opentelemetry/context/context.h @@ -3,7 +3,7 @@ #pragma once -#include +#include #include #include "opentelemetry/context/context_value.h" @@ -34,7 +34,7 @@ class Context // Creates a context object from a key and value, this will // hold a shared_ptr to the head of the DataList linked list - Context(nostd::string_view key, ContextValue value) noexcept + Context(nostd::string_view key, const ContextValue &value) noexcept : head_{nostd::shared_ptr{new DataList(key, value)}} {} @@ -42,10 +42,10 @@ class Context // contains the new key and value data. It attaches the // exisiting list to the end of the new list. template - Context SetValues(T &values) noexcept + Context SetValues(T &values) const noexcept { - Context context = Context(values); - nostd::shared_ptr last = context.head_; + Context context(values); + auto last = context.head_; while (last->next_ != nullptr) { last = last->next_; @@ -57,9 +57,9 @@ class Context // Accepts a new iterable and then returns a new context that // contains the new key and value data. It attaches the // exisiting list to the end of the new list. - Context SetValue(nostd::string_view key, ContextValue value) noexcept + Context SetValue(nostd::string_view key, const ContextValue &value) const noexcept { - Context context = Context(key, value); + Context context(key, value); context.head_->next_ = head_; return context; } @@ -69,12 +69,9 @@ class Context { for (DataList *data = head_.get(); data != nullptr; data = data->next_.get()) { - if (key.size() == data->key_length_) + if (key == data->key_) { - if (std::memcmp(key.data(), data->key_, data->key_length_) == 0) - { - return data->value_; - } + return data->value_; } } return ContextValue{}; @@ -92,12 +89,10 @@ class Context // A linked list to contain the keys and values of this context node struct DataList { - char *key_ = nullptr; + std::string key_; nostd::shared_ptr next_{nullptr}; - size_t key_length_ = 0UL; - ContextValue value_; DataList() = default; @@ -106,62 +101,23 @@ class Context template DataList(const T &keys_and_vals) { - bool first = true; + auto iter = std::begin(keys_and_vals); + if (iter == std::end(keys_and_vals)) + return; auto *node = this; - for (auto &iter : keys_and_vals) + *node = DataList(iter->first, iter->second); + for (++iter; iter != std::end(keys_and_vals); ++iter) { - if (first) - { - *node = DataList(iter.first, iter.second); - first = false; - } - else - { - node->next_ = nostd::shared_ptr(new DataList(iter.first, iter.second)); - node = node->next_.get(); - } + node->next_ = nostd::shared_ptr(new DataList(iter->first, iter->second)); + node = node->next_.get(); } } // Builds a data list with just a key and value, so it will just be the head // and returns that head. DataList(nostd::string_view key, const ContextValue &value) - { - key_ = new char[key.size()]; - key_length_ = key.size(); - std::memcpy(key_, key.data(), key.size() * sizeof(char)); - next_ = nostd::shared_ptr{nullptr}; - value_ = value; - } - - DataList(const DataList &other) - : key_(new char[other.key_length_]), - next_(other.next_), - key_length_(other.key_length_), - value_(other.value_) - { - std::memcpy(key_, other.key_, other.key_length_ * sizeof(char)); - } - - DataList &operator=(DataList &&other) noexcept - { - key_length_ = other.key_length_; - value_ = std::move(other.value_); - next_ = std::move(other.next_); - - key_ = other.key_; - other.key_ = nullptr; - - return *this; - } - - ~DataList() - { - if (key_ != nullptr) - { - delete[] key_; - } - } + : key_(key.begin(), key.end()), value_(value) + {} }; // Head of the list which holds the keys and values of this context diff --git a/api/include/opentelemetry/nostd/shared_ptr.h b/api/include/opentelemetry/nostd/shared_ptr.h index 681c4eb377..8d910dae13 100644 --- a/api/include/opentelemetry/nostd/shared_ptr.h +++ b/api/include/opentelemetry/nostd/shared_ptr.h @@ -11,7 +11,6 @@ #endif #if !defined(OPENTELEMETRY_HAVE_STD_SHARED_PTR) -# include # include # include @@ -32,13 +31,9 @@ class shared_ptr using pointer = element_type *; private: - static constexpr size_t kMaxSize = 32; static constexpr size_t kAlignment = 8; - struct alignas(kAlignment) PlacementBuffer - { - char data[kMaxSize]{}; - }; + struct alignas(kAlignment) PlacementBuffer; // fwd. class shared_ptr_wrapper { @@ -47,14 +42,12 @@ class shared_ptr shared_ptr_wrapper(std::shared_ptr &&ptr) noexcept : ptr_{std::move(ptr)} {} - virtual ~shared_ptr_wrapper() {} - - virtual void CopyTo(PlacementBuffer &buffer) const noexcept + void CopyTo(PlacementBuffer &buffer) const noexcept { new (buffer.data) shared_ptr_wrapper{*this}; } - virtual void MoveTo(PlacementBuffer &buffer) noexcept + void MoveTo(PlacementBuffer &buffer) noexcept { new (buffer.data) shared_ptr_wrapper{std::move(this->ptr_)}; } @@ -67,15 +60,19 @@ class shared_ptr new (buffer.data) other_shared_ptr_wrapper{std::move(this->ptr_)}; } - virtual pointer Get() const noexcept { return ptr_.get(); } + pointer Get() const noexcept { return ptr_.get(); } - virtual void Reset() noexcept { ptr_.reset(); } + void Reset() noexcept { ptr_.reset(); } private: std::shared_ptr ptr_; }; - static_assert(sizeof(shared_ptr_wrapper) <= kMaxSize, "Placement buffer is too small"); + struct alignas(kAlignment) PlacementBuffer + { + char data[sizeof(shared_ptr_wrapper)]{}; + }; + static_assert(alignof(shared_ptr_wrapper) <= kAlignment, "Placement buffer not properly aligned"); public: @@ -111,7 +108,7 @@ class shared_ptr shared_ptr(std::unique_ptr &&other) noexcept { - std::shared_ptr ptr_(other.release()); + std::shared_ptr ptr_(std::move(other)); new (buffer_.data) shared_ptr_wrapper{std::move(ptr_)}; } @@ -119,6 +116,10 @@ class shared_ptr shared_ptr &operator=(shared_ptr &&other) noexcept { + if (this == &other) + { + return *this; + } wrapper().~shared_ptr_wrapper(); other.wrapper().MoveTo(buffer_); return *this; @@ -132,6 +133,10 @@ class shared_ptr shared_ptr &operator=(const shared_ptr &other) noexcept { + if (this == &other) + { + return *this; + } wrapper().~shared_ptr_wrapper(); other.wrapper().CopyTo(buffer_); return *this; diff --git a/api/test/nostd/shared_ptr_test.cc b/api/test/nostd/shared_ptr_test.cc index fa3d97c493..505310206b 100644 --- a/api/test/nostd/shared_ptr_test.cc +++ b/api/test/nostd/shared_ptr_test.cc @@ -9,6 +9,7 @@ #include #include "opentelemetry/nostd/shared_ptr.h" +#include "opentelemetry/nostd/unique_ptr.h" using opentelemetry::nostd::shared_ptr; @@ -96,6 +97,15 @@ TEST(SharedPtrTest, MoveConstructionFromStdSharedPtr) EXPECT_EQ(ptr2.get(), value); } +TEST(SharedPtrTest, MoveConstructionFromNoStdUniquePtr) +{ + opentelemetry::v1::nostd::unique_ptr value(new int{123}); + auto p = value.get(); + shared_ptr ptr{std::move(value)}; + EXPECT_EQ(value.get(), nullptr); // NOLINT + EXPECT_EQ(ptr.get(), p); +} + TEST(SharedPtrTest, Destruction) { bool was_destructed;