Skip to content

Conversation

@aryansri05
Copy link

Changes or fixes:

This PR suppresses -Wshadow warnings in 13 header files when compiling with Clang on macOS.

The warnings were caused by local enumerator names (e.g., kBranchAny) shadowing global namespace enums. Since these are public API constants, they cannot be easily renamed without breaking backward compatibility.

Following the suggestion in the issue, I wrapped the affected code blocks with:

#if defined(__clang__)
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wshadow"
#endif
// ... code ...
#if defined(__clang__)
#pragma clang diagnostic pop
#endif
Checklist:
[x] tested changes locally

[ ] updated the docs (N/A)

This PR fixes #20790

@github-actions
Copy link

github-actions bot commented Jan 3, 2026

Test Results

    22 files      22 suites   3d 17h 20m 29s ⏱️
 3 791 tests  3 791 ✅ 0 💤 0 ❌
80 285 runs  80 285 ✅ 0 💤 0 ❌

Results for commit d6666b8.

Copy link
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

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

Thank you very much! However, I think the warnings only apply to parts of the header files, like the declaration of kBranchAny in TBranch.h, as explained in #20790.

Suppressing the warnings in the full header files is too drastic. The suppression should be local to the declarations where the warnings actually happen.

@aryansri05
Copy link
Author

@guitargeek I have updated the PR to apply the suppression only to the specific lines causing shadows, as requested.

Note: I have applied this surgical fix to 9 out of the 13 files. I temporarily reverted the other 4 (TVirtualX, TApplication, TGFrame, TSocket) to ensure the build passes for the majority. I can address those remaining 4 once I see the specific CI warnings for them in the next run.

@aryansri05
Copy link
Author

aryansri05 commented Jan 4, 2026

Update: I ran local diagnostics on the 4 reverted files (TApplication.h, TSocket.h, TVirtualX.h, TGFrame.h) using clang++ -std=c++17 -fsyntax-only -Wshadow on macOS.

They produced no shadow warnings. It appears the code in these files has changed since the original issue report and they are now clean. I will leave them untouched. This PR is ready for review.

@LukasBreitwieser LukasBreitwieser self-assigned this Jan 5, 2026
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.

4 participants