-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix: Suppress -Wshadow warnings in headers on macOS (Fixes #20790) #20793
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
base: master
Are you sure you want to change the base?
Fix: Suppress -Wshadow warnings in headers on macOS (Fixes #20790) #20793
Conversation
…ct#20790) Signed-off-by: Aryan Srivastava <[email protected]>
Test Results 22 files 22 suites 3d 17h 20m 29s ⏱️ Results for commit d6666b8. |
guitargeek
left a comment
There was a problem hiding this 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.
|
@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 ( |
|
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. |
Changes or fixes:
This PR suppresses
-Wshadowwarnings 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: