Skip to content

[CODE HEALTH] Fix clang-tidy narrowing conversions in baggage#3989

Merged
dbarker merged 2 commits intoopen-telemetry:mainfrom
thc1006:codehealth/baggage-narrowing
Apr 13, 2026
Merged

[CODE HEALTH] Fix clang-tidy narrowing conversions in baggage#3989
dbarker merged 2 commits intoopen-telemetry:mainfrom
thc1006:codehealth/baggage-narrowing

Conversation

@thc1006
Copy link
Copy Markdown
Contributor

@thc1006 thc1006 commented Apr 11, 2026

Fixes #3980
Contributes to #2053

Changes

  • Wrap three implicit int -> char conversions inside Baggage::UrlEncode
    and Baggage::UrlDecode with explicit static_cast<char>(...) so that
    cppcoreguidelines-narrowing-conversions stops firing under clang-tidy 20.
  • Decrement the clang-tidy.yaml warning_limit by 3 for both
    all-options-abiv1-preview and all-options-abiv2-preview matrix entries
    to ratchet the limit down to the new measured count.

The arithmetic is unchanged: c >> 4 and c & 15 are already bounded to
[0, 15] by the existing shift/mask, and the bitwise OR of two from_hex
nibbles in UrlDecode fits in a single byte. PR #1353 already added the same
static_cast<char> inside the from_hex lambda body back in 2022; this
change just finishes the job at the call sites that clang-tidy 20 newly
flags.

Fixes the following warnings:


opentelemetry-cpp/api/include/opentelemetry/baggage/baggage.h (3 warnings)

Line Check Message
238 cppcoreguidelines-narrowing-conversions narrowing conversion from 'int' to signed type 'char' is implementation-defined
239 cppcoreguidelines-narrowing-conversions narrowing conversion from 'int' to signed type 'char' is implementation-defined
270 cppcoreguidelines-narrowing-conversions narrowing conversion from 'int' to signed type 'char' is implementation-defined

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@thc1006 thc1006 requested a review from a team as a code owner April 11, 2026 09:47
Copilot AI review requested due to automatic review settings April 11, 2026 09:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Addresses new cppcoreguidelines-narrowing-conversions clang-tidy warnings in the Baggage URL encode/decode helpers and adjusts the clang-tidy CI threshold accordingly, keeping the project’s warning budget ratcheting downward.

Changes:

  • Add explicit static_cast<char>(...) at three previously-implicit int -> char conversion sites in Baggage::UrlEncode / Baggage::UrlDecode.
  • Lower warning_limit in the clang-tidy GitHub Actions workflow for the two preview matrix entries to match the reduced warning count.
  • Add an Unreleased changelog entry for the code health change.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
api/include/opentelemetry/baggage/baggage.h Makes narrowing conversions explicit to satisfy clang-tidy’s narrowing-conversions check.
.github/workflows/clang-tidy.yaml Updates warning thresholds to reflect the reduced clang-tidy warning count.
CHANGELOG.md Records the code health change in the Unreleased section.

Wrap three int->char conversions inside Baggage::UrlEncode and
Baggage::UrlDecode with explicit static_cast<char>(...) so that
cppcoreguidelines-narrowing-conversions stops firing on
api/include/opentelemetry/baggage/baggage.h:238,239,270 under
clang-tidy 20.

The change is purely a source-level cast: c >> 4 and c & 15 are
already bounded to [0, 15] by the surrounding mask/shift, and the
bitwise OR of two nibbles in UrlDecode fits in a single byte. The
existing baggage_test cases cover both UrlEncode and UrlDecode
round-trips and remain green.

Decrement the clang-tidy warning_limit by 3 for both
all-options-abiv1-preview and all-options-abiv2-preview matrix
entries to ratchet the limit down to the new measured count.

Resolves open-telemetry#3980. Part of open-telemetry#2053.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@thc1006 thc1006 force-pushed the codehealth/baggage-narrowing branch from 1e4b011 to 6a3e3c0 Compare April 11, 2026 09:55
@thc1006 thc1006 requested a review from Copilot April 11, 2026 10:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.17%. Comparing base (185dbd7) to head (31c4c4a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3989      +/-   ##
==========================================
- Coverage   90.18%   90.17%   -0.01%     
==========================================
  Files         230      230              
  Lines        7299     7299              
==========================================
- Hits         6582     6581       -1     
- Misses        717      718       +1     
Files with missing lines Coverage Δ
api/include/opentelemetry/baggage/baggage.h 98.28% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@dbarker dbarker left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the contribution!

Copy link
Copy Markdown
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix.

@dbarker dbarker merged commit 00e4d47 into open-telemetry:main Apr 13, 2026
68 checks passed
malkia added a commit to malkia/opentelemetry-cpp that referenced this pull request Apr 14, 2026
[CODE HEALTH] Fix clang-tidy narrowing conversions in baggage (open-telemetry#3989)
@thc1006 thc1006 deleted the codehealth/baggage-narrowing branch April 14, 2026 03:36
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.

[CODE HEALTH] clang-tidy warnings in baggage.h

4 participants