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

Improvement: span_id should not break strict aliasing. #1068

Merged
merged 2 commits into from
Nov 16, 2021

Conversation

hcoona
Copy link
Contributor

@hcoona hcoona commented Nov 16, 2021

Fixes #1067

@hcoona hcoona requested a review from a team November 16, 2021 04:12
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 16, 2021

CLA Signed

The committers are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Nov 16, 2021

Codecov Report

Merging #1068 (78695b8) into main (9772156) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1068   +/-   ##
=======================================
  Coverage   94.41%   94.41%           
=======================================
  Files         158      158           
  Lines        6077     6077           
=======================================
  Hits         5737     5737           
  Misses        340      340           
Impacted Files Coverage Δ
api/include/opentelemetry/trace/span_id.h 100.00% <100.00%> (ø)

@@ -61,7 +61,8 @@ class SpanId final
bool IsValid() const noexcept
{
static_assert(kSize == 8, "update is needed if kSize is not 8");
Copy link
Contributor

Choose a reason for hiding this comment

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

This static_assert could be removed then as the code doesn't make such assumption anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@hcoona hcoona force-pushed the private/hcoona/fix-strict-aliasing branch from 0259f8f to 0b9daa8 Compare November 16, 2021 05:37
@lalitb lalitb merged commit 69fdf67 into open-telemetry:main Nov 16, 2021
@hcoona hcoona deleted the private/hcoona/fix-strict-aliasing branch November 16, 2021 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API break strict aliasing.
3 participants