Fix: Suppress -Wshadow warnings in headers on macOS (Fixes #20790) #20793

aryansri05 · 2026-01-02T20:33:24Z

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

…ct#20790) Signed-off-by: Aryan Srivastava <your_github_email@example.com>

github-actions · 2026-01-03T23:34:32Z

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.

guitargeek

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.

… headers

aryansri05 · 2026-01-04T18:23:54Z

@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 · 2026-01-04T18:24:09Z

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.

core/base/inc/TApplication.h

pcanal · 2026-01-05T17:20:25Z

core/base/inc/TAttMarker.h

+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshadow"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif


Is the file wide setting still needed? For what symbols? (If it is needed, there should be clear inline documentation of why).

SIde note there should be nothing (new) before the copyright notice.

Thanks for the review. I've updated the PR to address your feedback:

Reverted TInterpreter.h: Removed the unnecessary changes there.

Corrected Placement: Moved all #pragma suppression blocks below the Copyright headers and includes to follow the file structure rules.

Added Documentation: Added inline comments (// FIXME) explaining that the file-wide suppression is needed due to legacy member variable shadowing.

Ready for re-review!

pcanal · 2026-01-05T17:20:47Z

core/base/inc/TSystem.h

+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshadow"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif


Is the file wide setting still needed? For what symbols? (If it is needed, there should be clear inline documentation of why).

SIde note there should be nothing (new) before the copyright notice.

Fixed (applied same pattern as above).

pcanal · 2026-01-05T17:20:53Z

core/meta/inc/TClass.h

+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshadow"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif


Is the file wide setting still needed? For what symbols? (If it is needed, there should be clear inline documentation of why).

SIde note there should be nothing (new) before the copyright notice.

Fixed (applied same pattern as above).

pcanal · 2026-01-05T17:21:12Z

core/meta/inc/TClass.h

+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshadow"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif


Why is his needed around a comment?

Fixed (applied same pattern as above).

pcanal · 2026-01-05T17:21:20Z

core/meta/inc/TInterpreter.h

+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshadow"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif


Is the file wide setting still needed? For what symbols? (If it is needed, there should be clear inline documentation of why).

SIde note there should be nothing (new) before the copyright notice.

Fixed (applied same pattern as above).

pcanal · 2026-01-05T17:21:38Z

math/matrix/inc/TMatrixT.h

+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshadow"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif


Is the file wide setting still needed? For what symbols? (If it is needed, there should be clear inline documentation of why).

SIde note there should be nothing (new) before the copyright notice.

Fixed (applied same pattern as above).

pcanal · 2026-01-05T17:21:44Z

math/matrix/inc/TMatrixTSparse.h

+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshadow"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif


Is the file wide setting still needed? For what symbols? (If it is needed, there should be clear inline documentation of why).

SIde note there should be nothing (new) before the copyright notice.

Fixed (applied same pattern as above).

pcanal · 2026-01-05T17:21:49Z

math/matrix/inc/TMatrixTSym.h

+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshadow"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif


Is the file wide setting still needed? For what symbols? (If it is needed, there should be clear inline documentation of why).

SIde note there should be nothing (new) before the copyright notice.

Fixed (applied same pattern as above).

pcanal · 2026-01-05T17:21:54Z

tree/tree/inc/TBranch.h

+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshadow"
+#elif defined(__GNUC__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif


Is the file wide setting still needed? For what symbols? (If it is needed, there should be clear inline documentation of why).

SIde note there should be nothing (new) before the copyright notice.

Fixed (applied same pattern as above).

ferdymercury · 2026-01-05T22:11:55Z

maybe it's worth to add the -Wshadow flag to the CI builds that have the -Woverride, so that the current fix stays maintained.

aryansri05 · 2026-01-07T18:34:41Z

@ferdymercury Apologies for the delay!

That’s a great suggestion regarding the CI. However, I’m concerned that enabling -Wshadow globally right now might trigger warnings in older parts of the codebase that I haven't touched, which would block this PR.

To keep this manageable, could we merge these fixes first? I’d be happy to open a follow-up PR to investigate enabling -Wshadow in the CI and fixing any remaining regressions separately.

pcanal · 2026-01-07T18:53:57Z

core/base/inc/TAttMarker.h

 #include "Rtypes.h"

+// FIXME: Temporarily suppress -Wshadow file-wide to avoid warnings from 
+// legacy member variables shadowing local variables (PR #20793).


What is the list shadowed variables in this file?

Hi @pcanal ,

In TAttMarker.h, the shadowed variables are the constructor arguments color and style.

I suppressed the warning because these argument names are part of the public API/header. I wasn't sure if renaming them (e.g. to mcolor, mstyle) would affect documentation or bindings. If you confirm it is safe to rename them in the header, I will do so and remove the pragma.

Thanks for the clarification, please provide the same information for all the files where there is still a global setting.

In TAttMarker.h, the shadowed variables are the constructor arguments color and style.

If it is only one function, why is the setting needed to be global to the file rather than around the constructor?

the constructor arguments color and style.

Out of curiosity where are the version that are shadowed declared?

suppressed the warning because these argument names are part of the public API/header. I wasn't sure if renaming them (e.g. to mcolor, mstyle) would affect documentation or bindings.

The names of the arguments are not 'important' and will not affect the binding. You are right that you also need to review the documentation of that/those function(s) to make sure they are updated accordingly.

Thanks for the clarification regarding the bindings. Since renaming the arguments won't affect the public API or bindings, I agree that renaming is the cleaner solution compared to suppressing warnings.

To answer your questions:

Files involved: The files where I had applied the global suppression (which I will now revert in favor of renaming) are TAttMarker.h, TAttLine.h, TAttFill.h, and TAttText.h.

Why global: I originally placed the suppression globally to avoid cluttering the class definitions with #pragma directives inside the header files. However, renaming the arguments eliminates the need for suppression entirely, which is much better.

Where they are shadowed: The shadowing usually occurs when these headers are included in an environment where global variables like color, style, or font are defined (often from system headers, X11 libraries, or user macros).

Plan: I will proceed with renaming the constructor arguments in the header and source files (e.g., color -> markerColor, style -> markerStyle) to resolve the shadowing warnings and remove the global suppressions. I will also ensure the corresponding documentation/comments are updated to match the new argument names.

pcanal · 2026-01-07T19:47:35Z

core/base/inc/TAttMarker.h

+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wshadow"
+#endif
   enum EMarkerStyle {kDot=1, kPlus, kStar, kCircle=4, kMultiply=5,


Please document exactly which of the enums constant are the one shadowing existing symbols and list/refer where the shadowed symbols comes from (if known).

The suppression was intended for the TAttMarker constructor arguments (color and style), not these enums. However, since we established that renaming the arguments is safe, this suppression block is no longer necessary. I will remove it and rename the arguments in the next update

This is puzzling since you put these guard specifically around this lines and thus it must/might have been related to one of the symbol on that line, isn't it?

Fix: Suppress -Wshadow warnings in headers on macOS (Fixes root-proje…

34a0eee

…ct#20790) Signed-off-by: Aryan Srivastava <your_github_email@example.com>

aryansri05 requested review from bellenot, dpiparo, lmoneta and pcanal as code owners January 2, 2026 20:33

aryansri05 mentioned this pull request Jan 2, 2026

Applications compiled on MacOS with -Wshadow report warnings from 14 ROOT headers #20790

Open

1 task

Style: Remove extra newline before footer to satisfy clang-format

d6666b8

guitargeek requested changes Jan 4, 2026

View reviewed changes

Refactor: Narrow scope of -Wshadow suppression to specific enums in 9…

aefdf3f

… headers

ferdymercury reviewed Jan 5, 2026

View reviewed changes

core/base/inc/TApplication.h Outdated Show resolved Hide resolved

Revert 4 files to original upstream state

4698a8c

LukasBreitwieser self-assigned this Jan 5, 2026

pcanal reviewed Jan 5, 2026

View reviewed changes

Refactor: Add GCC support for -Wshadow suppression

b412441

aryansri05 force-pushed the fix-macos-shadow-warnings branch from 8403b42 to b412441 Compare January 5, 2026 17:38

pcanal reviewed Jan 7, 2026

View reviewed changes

Fix: Suppress -Wshadow warnings in headers on macOS (Fixes #20790) #20793

Are you sure you want to change the base?

Fix: Suppress -Wshadow warnings in headers on macOS (Fixes #20790) #20793

Conversation

aryansri05 commented Jan 2, 2026

Changes or fixes:

Uh oh!

github-actions bot commented Jan 3, 2026

Test Results

Uh oh!

guitargeek left a comment

Choose a reason for hiding this comment

Uh oh!

aryansri05 commented Jan 4, 2026

Uh oh!

aryansri05 commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferdymercury commented Jan 5, 2026

Uh oh!

aryansri05 commented Jan 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

aryansri05 commented Jan 4, 2026 •

edited

Loading