Skip to content

Commit ebdb633

Browse files
authored
[CODE HEALTH] Fix clang tidy warnings in API common and context (#3948)
1 parent 06f6086 commit ebdb633

File tree

12 files changed

+93
-45
lines changed

12 files changed

+93
-45
lines changed

.clang-tidy

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ Checks: >
55
-*,
66
performance-*,
77
portability-*,
8+
-portability-template-virtual-member-function,
89
abseil-*,
910
-abseil-string-find-str-contains,
1011
bugprone-*,

.github/workflows/clang-tidy.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ jobs:
1717
matrix:
1818
include:
1919
- cmake_options: all-options-abiv1-preview
20-
warning_limit: 204
20+
warning_limit: 170
2121
- cmake_options: all-options-abiv2-preview
22-
warning_limit: 206
22+
warning_limit: 166
2323
env:
2424
CC: /usr/bin/clang-18
2525
CXX: /usr/bin/clang++-18

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ Increment the:
1515

1616
## [Unreleased]
1717

18+
* [CODE HEALTH] Fix clang tidy warnings in API `common` and `context`
19+
[#3948](https://github.com/open-telemetry/opentelemetry-cpp/pull/3948)
20+
1821
## [1.26.0] 2026-03-19
1922

2023
* [RELEASE] Bump main branch to 1.26.0-dev

api/include/opentelemetry/common/key_value_iterable.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,12 @@ namespace common
1717
class KeyValueIterable
1818
{
1919
public:
20-
virtual ~KeyValueIterable() = default;
20+
KeyValueIterable() = default;
21+
KeyValueIterable(const KeyValueIterable &) = default;
22+
KeyValueIterable &operator=(const KeyValueIterable &) = default;
23+
KeyValueIterable(KeyValueIterable &&) noexcept = default;
24+
KeyValueIterable &operator=(KeyValueIterable &&) noexcept = default;
25+
virtual ~KeyValueIterable() = default;
2126

2227
/**
2328
* Iterate over key-value pairs
@@ -40,8 +45,6 @@ class KeyValueIterable
4045
class NoopKeyValueIterable : public KeyValueIterable
4146
{
4247
public:
43-
~NoopKeyValueIterable() override = default;
44-
4548
/**
4649
* No-op implementation: does not invoke the callback, even if key-value pairs are present.
4750
* @return true without iterating or invoking the callback

api/include/opentelemetry/common/key_value_iterable_view.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ MakeAttributes(
122122
* @return common::KeyValueIterable
123123
*/
124124
inline static const common::KeyValueIterable &MakeAttributes(
125-
const common::KeyValueIterable &attributes) noexcept
125+
const common::KeyValueIterable &attributes OPENTELEMETRY_ATTRIBUTE_LIFETIME_BOUND) noexcept
126126
{
127127
return attributes;
128128
}

api/include/opentelemetry/common/kv_properties.h

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class KeyValueStringTokenizer
3333
KeyValueStringTokenizer(
3434
nostd::string_view str,
3535
const KeyValueStringTokenizerOptions &opts = KeyValueStringTokenizerOptions()) noexcept
36-
: str_(str), opts_(opts), index_(0)
36+
: str_(str), opts_(opts)
3737
{}
3838

3939
static nostd::string_view GetDefaultKeyOrValue()
@@ -129,7 +129,7 @@ class KeyValueStringTokenizer
129129
private:
130130
nostd::string_view str_;
131131
KeyValueStringTokenizerOptions opts_;
132-
size_t index_;
132+
size_t index_{};
133133
};
134134

135135
// Class to store fixed size array of key-value pairs of string type
@@ -140,7 +140,8 @@ class KeyValueProperties
140140
class Entry
141141
{
142142
public:
143-
Entry() : key_(nullptr), value_(nullptr) {}
143+
Entry() = default;
144+
~Entry() = default;
144145

145146
// Copy constructor
146147
Entry(const Entry &copy)
@@ -150,16 +151,20 @@ class KeyValueProperties
150151
}
151152

152153
// Copy assignment operator
153-
Entry &operator=(Entry &other)
154+
Entry &operator=(const Entry &other)
154155
{
156+
if (this == &other)
157+
{
158+
return *this;
159+
}
155160
key_ = CopyStringToPointer(other.key_.get());
156161
value_ = CopyStringToPointer(other.value_.get());
157162
return *this;
158163
}
159164

160165
// Move contructor and assignment operator
161-
Entry(Entry &&other) = default;
162-
Entry &operator=(Entry &&other) = default;
166+
Entry(Entry &&other) noexcept = default;
167+
Entry &operator=(Entry &&other) noexcept = default;
163168

164169
// Creates an Entry for a given key-value pair.
165170
Entry(nostd::string_view key, nostd::string_view value)
@@ -194,29 +199,25 @@ class KeyValueProperties
194199
};
195200

196201
// Maintain the number of entries in entries_.
197-
size_t num_entries_;
202+
size_t num_entries_{};
198203

199204
// Max size of allocated array
200-
size_t max_num_entries_;
205+
size_t max_num_entries_{};
201206

202207
// Store entries in a C-style array to avoid using std::array or std::vector.
203208
nostd::unique_ptr<Entry[]> entries_;
204209

205210
public:
206211
// Create Key-value list of given size
207212
// @param size : Size of list.
208-
KeyValueProperties(size_t size) noexcept
209-
: num_entries_(0), max_num_entries_(size), entries_(new Entry[size])
210-
{}
213+
KeyValueProperties(size_t size) noexcept : max_num_entries_(size), entries_(new Entry[size]) {}
211214

212215
// Create Empty Key-Value list
213-
KeyValueProperties() noexcept : num_entries_(0), max_num_entries_(0), entries_(nullptr) {}
216+
KeyValueProperties() noexcept = default;
214217

215218
template <class T, class = typename std::enable_if<detail::is_key_value_iterable<T>::value>::type>
216219
KeyValueProperties(const T &keys_and_values) noexcept
217-
: num_entries_(0),
218-
max_num_entries_(keys_and_values.size()),
219-
entries_(new Entry[max_num_entries_])
220+
: max_num_entries_(keys_and_values.size()), entries_(new Entry[max_num_entries_])
220221
{
221222
for (auto &e : keys_and_values)
222223
{

api/include/opentelemetry/common/spin_lock_mutex.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ class SpinLockMutex
5656
~SpinLockMutex() noexcept = default;
5757
SpinLockMutex(const SpinLockMutex &) = delete;
5858
SpinLockMutex &operator=(const SpinLockMutex &) = delete;
59+
SpinLockMutex(SpinLockMutex &&) = delete;
60+
SpinLockMutex &operator=(SpinLockMutex &&) = delete;
5961

6062
static inline void fast_yield() noexcept
6163
{

api/include/opentelemetry/context/context.h

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class Context
4444
template <class T>
4545
Context SetValues(T &values) noexcept
4646
{
47-
Context context = Context(values);
47+
Context context(values);
4848
nostd::shared_ptr<DataList> last = context.head_;
4949
while (last->next_ != nullptr)
5050
{
@@ -59,7 +59,7 @@ class Context
5959
// exisiting list to the end of the new list.
6060
Context SetValue(nostd::string_view key, ContextValue value) noexcept
6161
{
62-
Context context = Context(key, std::move(value));
62+
Context context(key, std::move(value));
6363
context.head_->next_ = head_;
6464
return context;
6565
}
@@ -92,11 +92,11 @@ class Context
9292
// A linked list to contain the keys and values of this context node
9393
struct DataList
9494
{
95-
char *key_ = nullptr;
95+
char *key_{nullptr};
9696

9797
nostd::shared_ptr<DataList> next_{nullptr};
9898

99-
size_t key_length_ = 0UL;
99+
size_t key_length_{};
100100

101101
ContextValue value_;
102102

@@ -108,7 +108,7 @@ class Context
108108
{
109109
bool first = true;
110110
auto *node = this;
111-
for (auto &iter : keys_and_vals)
111+
for (const auto &iter : keys_and_vals)
112112
{
113113
if (first)
114114
{
@@ -126,29 +126,28 @@ class Context
126126
// Builds a data list with just a key and value, so it will just be the head
127127
// and returns that head.
128128
DataList(nostd::string_view key, ContextValue value)
129+
: key_{new char[key.size()]},
130+
next_{nostd::shared_ptr<DataList>{nullptr}},
131+
key_length_{key.size()},
132+
value_{std::move(value)}
129133
{
130-
key_ = new char[key.size()];
131-
key_length_ = key.size();
132134
std::memcpy(key_, key.data(), key.size() * sizeof(char));
133-
next_ = nostd::shared_ptr<DataList>{nullptr};
134-
value_ = std::move(value);
135135
}
136136

137-
DataList(const DataList &other)
138-
: key_(new char[other.key_length_]),
139-
next_(other.next_),
140-
key_length_(other.key_length_),
141-
value_(other.value_)
142-
{
143-
std::memcpy(key_, other.key_, other.key_length_ * sizeof(char));
144-
}
137+
// Delete unused special member functions
138+
DataList(const DataList &other) = delete;
139+
DataList(DataList &&other) noexcept = delete;
140+
DataList &operator=(const DataList &other) = delete;
145141

142+
// Move assignment operator is only called by DataList(const T &keys_and_vals)
143+
// The moved to object has default member values
146144
DataList &operator=(DataList &&other) noexcept
147145
{
148146
key_length_ = other.key_length_;
149147
value_ = std::move(other.value_);
150148
next_ = std::move(other.next_);
151149

150+
// key_ has a default nullptr value and does not need to be deleted before this assignment
152151
key_ = other.key_;
153152
other.key_ = nullptr;
154153

api/include/opentelemetry/context/propagation/text_map_propagator.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ namespace propagation
1818
class TextMapCarrier
1919
{
2020
public:
21+
TextMapCarrier() = default;
22+
TextMapCarrier(const TextMapCarrier &) = default;
23+
TextMapCarrier &operator=(const TextMapCarrier &) = default;
24+
TextMapCarrier(TextMapCarrier &&) noexcept = default;
25+
TextMapCarrier &operator=(TextMapCarrier &&) noexcept = default;
26+
2127
// returns the value associated with the passed key.
2228
virtual nostd::string_view Get(nostd::string_view key) const noexcept = 0;
2329

@@ -42,6 +48,12 @@ class TextMapCarrier
4248
class TextMapPropagator
4349
{
4450
public:
51+
TextMapPropagator() = default;
52+
TextMapPropagator(const TextMapPropagator &) = default;
53+
TextMapPropagator &operator=(const TextMapPropagator &) = default;
54+
TextMapPropagator(TextMapPropagator &&) noexcept = default;
55+
TextMapPropagator &operator=(TextMapPropagator &&) noexcept = default;
56+
4557
// Returns the context that is stored in the carrier with the TextMapCarrier as extractor.
4658
virtual context::Context Extract(const TextMapCarrier &carrier,
4759
context::Context &context) noexcept = 0;

api/include/opentelemetry/context/runtime_context.h

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ namespace context
2424
class Token
2525
{
2626
public:
27+
Token(const Token &) = delete;
28+
Token &operator=(const Token &) = delete;
29+
Token(Token &&) = delete;
30+
Token &operator=(Token &&) = delete;
31+
2732
bool operator==(const Context &other) const noexcept { return context_ == other; }
2833

2934
virtual ~Token() noexcept;
@@ -35,6 +40,8 @@ class Token
3540
// one that was passed in.
3641
Token(const Context &context) : context_(context) {}
3742

43+
// Keep the context_ member as const to ensure the Token is immutable.
44+
// NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members)
3845
const Context context_;
3946
};
4047

@@ -48,6 +55,13 @@ class Token
4855
class OPENTELEMETRY_EXPORT RuntimeContextStorage
4956
{
5057
public:
58+
RuntimeContextStorage() = default;
59+
// Keep RuntimeContextStorage copyable but not movable to preserve the existing API.
60+
RuntimeContextStorage(const RuntimeContextStorage &) = default;
61+
RuntimeContextStorage &operator=(const RuntimeContextStorage &) = default;
62+
RuntimeContextStorage(RuntimeContextStorage &&) noexcept = delete;
63+
RuntimeContextStorage &operator=(RuntimeContextStorage &&) noexcept = delete;
64+
5165
/**
5266
* Return the current context.
5367
* @return the current context
@@ -68,7 +82,7 @@ class OPENTELEMETRY_EXPORT RuntimeContextStorage
6882
*/
6983
virtual bool Detach(Token &token) noexcept = 0;
7084

71-
virtual ~RuntimeContextStorage() {}
85+
virtual ~RuntimeContextStorage() = default;
7286

7387
protected:
7488
nostd::unique_ptr<Token> CreateToken(const Context &context) noexcept
@@ -261,7 +275,11 @@ class ThreadLocalContextStorage : public RuntimeContextStorage
261275
*/
262276
static constexpr size_t max_capacity_ = 1000000;
263277

264-
Stack() noexcept : size_(0), capacity_(0), base_(nullptr) {}
278+
Stack() noexcept = default;
279+
Stack(const Stack &) = delete;
280+
Stack &operator=(const Stack &) = delete;
281+
Stack(Stack &&) = delete;
282+
Stack &operator=(Stack &&) = delete;
265283

266284
// Pops the top Context off the stack.
267285
void Pop() noexcept
@@ -359,14 +377,14 @@ class ThreadLocalContextStorage : public RuntimeContextStorage
359377

360378
~Stack() noexcept { delete[] base_; }
361379

362-
size_t size_;
363-
size_t capacity_;
364-
Context *base_;
380+
size_t size_{};
381+
size_t capacity_{};
382+
Context *base_{nullptr};
365383
};
366384

367385
OPENTELEMETRY_API_SINGLETON Stack &GetStack()
368386
{
369-
static thread_local Stack stack_ = Stack();
387+
static thread_local Stack stack_{};
370388
return stack_;
371389
}
372390
};

0 commit comments

Comments
 (0)