Skip to content

[CODE HEALTH] Fix clang-tidy misc-override-with-different-visibility warnings#4215

Merged
marcalff merged 4 commits into
open-telemetry:mainfrom
thc1006:codehealth/misc-override-visibility-4195
Jul 7, 2026
Merged

[CODE HEALTH] Fix clang-tidy misc-override-with-different-visibility warnings#4215
marcalff merged 4 commits into
open-telemetry:mainfrom
thc1006:codehealth/misc-override-visibility-4195

Conversation

@thc1006

@thc1006 thc1006 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Contributes to #4195

Changes

Fixes a subset of #4195 by aligning the visibility of virtual function overrides with their base-class declaration, which is the fix the misc-override-with-different-visibility check asks for (per the check documentation).

This PR resolves 31 of the 36 reported warnings across 13 files. The remaining 5 warnings (in ext/include/opentelemetry/ext/http/server/http_server.h and socket_tools.h) arise from private/protected inheritance of SocketTools::Reactor::SocketCallback and need a different, more careful change; they are deferred to a follow-up PR to keep this one reviewable.

Fix pattern

  • Base public -> derived override made public (widening; e.g. test mocks of GetLogger/GetTracer, Predicate::Match, Fields)
  • Base protected -> derived override made protected (e.g. sdk::logs::Logger::EnabledImplementation, gtest SetUp/TearDown)
  • Base private (NVI) -> derived override made private (e.g. MetricReader::OnForceFlush/OnShutDown/OnInitialized)

No behavior change: only access specifiers on overrides are adjusted; .cc definitions do not restate visibility.

Verification

  • Built all affected test and example targets locally (clang-22, ABI v1 and v2 presets); all compile and link.
  • clang-format clean on all changed files.
  • The warning_limit in .github/workflows/clang-tidy.yaml is lowered accordingly (abiv1: 309 -> 280, abiv2: 319 -> 288), derived from the current CI baseline minus the 29 (abiv1) / 31 (abiv2) warnings resolved here.

…warnings

Align the visibility of virtual overrides with their base-class
declaration across 13 files, resolving 31 of the 36 warnings from open-telemetry#4195.
The remaining 5 (http_server.h / socket_tools.h private-inheritance
cases) are deferred to a follow-up. Lower the clang-tidy warning_limit
to 280 (abiv1) and 288 (abiv2) accordingly.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 6, 2026 14:38
@thc1006 thc1006 requested a review from a team as a code owner July 6, 2026 14:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@thc1006 thc1006 requested a review from Copilot July 6, 2026 14:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.83%. Comparing base (db83a82) to head (d340a9f).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4215   +/-   ##
=======================================
  Coverage   82.83%   82.83%           
=======================================
  Files         415      415           
  Lines       17431    17431           
=======================================
  Hits        14437    14437           
  Misses       2994     2994           
Files with missing lines Coverage Δ
...entelemetry/trace/propagation/http_trace_context.h 92.86% <ø> (ø)
...plar/aligned_histogram_bucket_exemplar_reservoir.h 86.67% <ø> (ø)
...include/opentelemetry/sdk/metrics/view/predicate.h 91.67% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marcalff marcalff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the cleanup

@marcalff marcalff merged commit 4b58597 into open-telemetry:main Jul 7, 2026
72 checks passed
@thc1006 thc1006 deleted the codehealth/misc-override-visibility-4195 branch July 7, 2026 17:07
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.

3 participants