Skip to content

[CODE HEALTH] Fix clang-tidy cppcoreguidelines-special-member-functions warnings in trace API #3934

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

Open
dbarker wants to merge 2 commits into
base: main
Choose a base branch
from
+36 −9
Open

[CODE HEALTH] Fix clang-tidy cppcoreguidelines-special-member-functions warnings in trace API #3934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed586ee
Fix clang-tidy trace special member functions
dbarker Mar 14, 2026
21622f3
Merge branch 'main' into fix_clang_tidy_trace_special_member_functions
marcalff Mar 14, 2026
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
19 changes: 17 additions & 2 deletions api/include/opentelemetry/trace/default_span.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ namespace trace
class DefaultSpan : public Span
{
public:
~DefaultSpan() noexcept override = default;

// Returns an invalid span.
static DefaultSpan GetInvalid() { return DefaultSpan(SpanContext::GetInvalid()); }

Expand Down Expand Up @@ -66,8 +68,21 @@ class DefaultSpan : public Span
DefaultSpan(SpanContext span_context) noexcept : span_context_(std::move(span_context)) {}

// movable and copiable
DefaultSpan(DefaultSpan &&spn) noexcept : Span(), span_context_(spn.GetContext()) {}
DefaultSpan(const DefaultSpan &spn) noexcept : Span(), span_context_(spn.GetContext()) {}
DefaultSpan(const DefaultSpan &spn) noexcept : span_context_(spn.span_context_) {}

DefaultSpan &operator=(const DefaultSpan &spn)
{
Copy link
Member

@marcalff marcalff Mar 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure, should the code test assignment of a DefaultSpan to itself ?

Copy link
Member Author

@dbarker dbarker Mar 15, 2026

Choose a reason for hiding this comment

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

Good question. I don't think we really need a self-assignment test here. DefaultSpan assignment only assigns SpanContext, and SpanContext is made of self-safe value members.

Copy link
Member

@marcalff marcalff Mar 15, 2026

Choose a reason for hiding this comment

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

@dbarker

Thanks for the investigations.

Please add a self-assignment test then, for robustness, so we don't have to think about it again.

This code is not likely to be in any critical path anyway.

span_context_ = spn.span_context_;
return *this;
}

DefaultSpan(DefaultSpan &&spn) noexcept : span_context_(std::move(spn.span_context_)) {}

DefaultSpan &operator=(DefaultSpan &&spn) noexcept
{
span_context_ = std::move(spn.span_context_);
return *this;
}

private:
SpanContext span_context_;
Expand Down
4 changes: 0 additions & 4 deletions api/include/opentelemetry/trace/span_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ class SpanContext final
trace_state_(std::move(trace_state))
{}

SpanContext(const SpanContext &ctx) = default;

// @returns whether this context is valid
bool IsValid() const noexcept { return trace_id_.IsValid() && span_id_.IsValid(); }

Expand All @@ -79,8 +77,6 @@ class SpanContext final
trace_flags() == that.trace_flags();
}

SpanContext &operator=(const SpanContext &ctx) = default;

bool IsRemote() const noexcept { return is_remote_; }

static SpanContext GetInvalid() noexcept { return SpanContext(false, false); }
Expand Down
7 changes: 6 additions & 1 deletion api/include/opentelemetry/trace/span_context_kv_iterable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ namespace trace
class SpanContextKeyValueIterable
{
public:
virtual ~SpanContextKeyValueIterable() = default;
SpanContextKeyValueIterable() = default;
virtual ~SpanContextKeyValueIterable() = default;
SpanContextKeyValueIterable(const SpanContextKeyValueIterable &) = default;
SpanContextKeyValueIterable &operator=(const SpanContextKeyValueIterable &) = default;
SpanContextKeyValueIterable(SpanContextKeyValueIterable &&) = default;
SpanContextKeyValueIterable &operator=(SpanContextKeyValueIterable &&) = default;

/**
* Iterate over SpanContext/key-value pairs
Expand Down
8 changes: 7 additions & 1 deletion api/include/opentelemetry/trace/tracer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ namespace trace
class Tracer
{
public:
virtual ~Tracer() = default;
Tracer() = default;
virtual ~Tracer() = default;
Tracer(const Tracer &) = default;
Tracer &operator=(const Tracer &) = default;
Tracer(Tracer &&) = default;
Tracer &operator=(Tracer &&) = default;

/**
* Starts a span.
*
Expand Down
7 changes: 6 additions & 1 deletion api/include/opentelemetry/trace/tracer_provider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ class Tracer;
class OPENTELEMETRY_EXPORT TracerProvider
{
public:
virtual ~TracerProvider() = default;
TracerProvider() = default;
virtual ~TracerProvider() = default;
TracerProvider(const TracerProvider &) = default;
TracerProvider &operator=(const TracerProvider &) = default;
TracerProvider(TracerProvider &&) = default;
TracerProvider &operator=(TracerProvider &&) = default;

#if OPENTELEMETRY_ABI_VERSION_NO >= 2

Expand Down
Loading