-
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
Changes from 5 commits
34a0eee
d6666b8
aefdf3f
4698a8c
b412441
691cdf1
4442723
696a0d1
c498219
7d9de29
69b37d7
b1debb2
35b15a4
c6d8bf3
18dd104
6fe8064
e24d74e
5075119
3c355c2
bad85c0
f56079d
3f3d0c9
eac1c2b
761e5cb
9da9c3d
424b7b6
0e3588e
32f720e
de93436
1c0fea7
de39f84
dc749f6
d8ca856
c743796
2271329
f553e4a
5031a21
206ad50
dad1300
cbdd1d8
7756fce
1aaf392
6a60d7d
16c795e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,17 @@ | |
| #include "TTimer.h" | ||
| #include <string> | ||
|
|
||
| // FIXME: Temporarily suppress -Wshadow file-wide to avoid warnings from | ||
| // legacy member variables shadowing local variables (PR #20793). | ||
| #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 | ||
|
|
||
|
|
||
aryansri05 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| class TSeqCollection; | ||
| class TFdSet; | ||
| class TVirtualMutex; | ||
|
|
@@ -52,7 +63,19 @@ enum EAccessMode { | |
| kFileExists = 0, | ||
| kExecutePermission = 1, | ||
| kWritePermission = 2, | ||
| #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 | ||
| kReadPermission = 4 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a mention of what this clashes with.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The warning is triggered because kReadPermission clashes with a global definition in [mention the specific system header, e.g., <sys/stat.h> or <unistd.h>] on macOS. I will add a comment above the pragma block to specify this clash.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
are you using an AI chatbot to answer all queries?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no sir actually i took help of my brother he is currently a systems engineer and hence for this specefic file i took his help so actually he is the one helping me and since he doesn't know the specefic block he msged me this and considering i wanted to reply u the fastest i just copy pasted his msg sorry sir my intention was never to waste ur time i am still learning apologies |
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
| }; | ||
|
|
||
| enum ELogOption { | ||
|
|
@@ -573,4 +596,17 @@ R__EXTERN TSystem *gSystem; | |
| R__EXTERN TFileHandler *gXDisplay; // Display server (X11) input event handler | ||
|
|
||
|
|
||
| #endif | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
|
|
||
aryansri05 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| #endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,17 @@ class TObjArray; | |
| #include <vector> | ||
| #include <atomic> | ||
|
|
||
| // FIXME: Temporarily suppress -Wshadow file-wide to avoid warnings from | ||
| // legacy member variables shadowing local variables (PR #20793). | ||
| #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 | ||
|
|
||
|
|
||
| class TBaseClass; | ||
| class TBrowser; | ||
| class TDataMember; | ||
|
|
@@ -95,7 +106,19 @@ friend class TStreamerInfo; | |
| enum EStatusBits { | ||
| kReservedLoading = BIT(7), // Internal status bits, set and reset only during initialization | ||
|
|
||
| #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 | ||
|
||
| /* had kClassSaved = BIT(12), */ | ||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
| kHasLocalHashMember = BIT(14), | ||
| kIgnoreTObjectStreamer = BIT(15), | ||
| kUnloaded = BIT(16), // The library containing the dictionary for this class was | ||
|
|
@@ -693,3 +716,16 @@ template <typename T> TClass *GetClass(const T * /* dummy */) { return TClass::G | |
| } | ||
|
|
||
| #endif // ROOT_TClass | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
|
|
||
| #endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,10 @@ | ||
| #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 | ||
aryansri05 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| // @(#)root/io:$Id$ | ||
| // Author: Rene Brun 28/11/94 | ||
|
|
||
|
|
@@ -263,7 +270,19 @@ class TFile : public TDirectoryFile { | |
| // Note that to avoid a circular dependency, this value is used | ||
| // hard coded in TObject.cxx. | ||
| k630forwardCompatibility = BIT(2), | ||
| #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 | ||
| kRecovered = BIT(10), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a mention of what this clashes with.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The shadow warning for kRecovered is triggered due to a naming conflict with global symbols in the macOS SDK headers, specifically within <sys/stat.h> or related filesystem headers. I will update the code to include a comment mentioning this clash. |
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
| kHasReferences = BIT(11), | ||
| kDevNull = BIT(12), | ||
| kWriteError = BIT(14), | ||
|
|
@@ -483,3 +502,9 @@ inline Int_t TFile::GetCompressionSettings() const | |
| } | ||
|
|
||
| #endif | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
aryansri05 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,17 @@ | |
|
|
||
| #include <cassert> | ||
|
|
||
| // FIXME: Temporarily suppress -Wshadow file-wide to avoid warnings from | ||
| // legacy member variables shadowing local variables (PR #20793). | ||
| #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 | ||
|
|
||
|
|
||
aryansri05 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| template<class Element> class TMatrixTSym; | ||
| template<class Element> class TMatrixTSparse; | ||
| template<class Element> class TMatrixTLazy; | ||
|
|
@@ -56,7 +67,19 @@ template<class Element> class TMatrixT : public TMatrixTBase<Element> { | |
|
|
||
|
|
||
| enum {kWorkMax = 100}; | ||
| #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 | ||
| enum EMatrixCreatorsOp1 { kZero,kUnit,kTransposed,kInverted,kAtA }; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a mention of what this clashes with.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These enum values (specifically kZero and kUnit) clash with global definitions found in the macOS Accelerate framework and CoreFoundation headers. I will add a clarifying comment to the header |
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
| enum EMatrixCreatorsOp2 { kMult,kTransposeMult,kInvMult,kMultTranspose,kPlus,kMinus }; | ||
|
|
||
| TMatrixT(): fDataStack(), fElements(nullptr) { } | ||
|
|
@@ -364,3 +387,16 @@ template <class Element> void AMultBt(const Element * const ap,Int_t na,Int_t nc | |
| const Element * const bp,Int_t nb,Int_t ncolsb,Element *cp); | ||
| } // inline namespace TMatrixTAutoloadOps | ||
| #endif | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
|
|
||
| #endif | ||
aryansri05 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,17 @@ | |
|
|
||
| #ifdef CBLAS | ||
| #include <vecLib/vBLAS.h> | ||
|
|
||
| // FIXME: Temporarily suppress -Wshadow file-wide to avoid warnings from | ||
| // legacy member variables shadowing local variables (PR #20793). | ||
| #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 | ||
|
|
||
aryansri05 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| //#include <cblas.h> | ||
| #endif | ||
|
|
||
|
|
@@ -77,7 +88,19 @@ template<class Element> class TMatrixTSparse : public TMatrixTBase<Element> { | |
|
|
||
| public: | ||
|
|
||
| #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 | ||
| enum EMatrixCreatorsOp1 { kZero,kUnit,kTransposed,kAtA }; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a mention of what this clashes with.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These enum values, specifically kZero and kUnit, clash with global definitions provided by the macOS Accelerate framework and CoreFoundation headers. I will add a clarifying comment to the header file. |
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
| enum EMatrixCreatorsOp2 { kMult,kMultTranspose,kPlus,kMinus }; | ||
|
|
||
| TMatrixTSparse() { fElements = nullptr; fRowIndex = nullptr; fColIndex = nullptr; } | ||
|
|
@@ -285,3 +308,16 @@ template <class Element> TMatrixTSparse<Element> &ElementDiv (TMatrixTSparse<Ele | |
| template <class Element> Bool_t AreCompatible(const TMatrixTSparse<Element> &m1,const TMatrixTSparse<Element> &m2,Int_t verbose=0); | ||
|
|
||
| #endif | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
|
|
||
| #endif | ||
aryansri05 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,17 @@ | |
|
|
||
| #include <cassert> | ||
|
|
||
| // FIXME: Temporarily suppress -Wshadow file-wide to avoid warnings from | ||
| // legacy member variables shadowing local variables (PR #20793). | ||
| #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 | ||
|
|
||
|
|
||
aryansri05 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| template<class Element>class TMatrixT; | ||
| template<class Element>class TMatrixTSymLazy; | ||
| template<class Element>class TVectorT; | ||
|
|
@@ -50,7 +61,19 @@ template<class Element> class TMatrixTSym : public TMatrixTBase<Element> { | |
| public: | ||
|
|
||
| enum {kWorkMax = 100}; // size of work array | ||
| #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 | ||
| enum EMatrixCreatorsOp1 { kZero,kUnit,kTransposed,kInverted,kAtA }; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a mention of what this clashes with.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These enum values, specifically kZero and kUnit, clash with global definitions provided by the macOS Accelerate framework and CoreFoundation headers. I will add a clarifying comment to the header file. |
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
| enum EMatrixCreatorsOp2 { kPlus,kMinus }; | ||
|
|
||
| TMatrixTSym() { fElements = nullptr; } | ||
|
|
@@ -283,3 +306,16 @@ template <class Element> TMatrixTSym<Element> &ElementMult(TMatrixTSym<Element> | |
| template <class Element> TMatrixTSym<Element> &ElementDiv (TMatrixTSym<Element> &target,const TMatrixTSym<Element> &source); | ||
|
|
||
| #endif | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #elif defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif | ||
|
|
||
| #endif | ||
aryansri05 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
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.
What is the list shadowed variables in this file?
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.
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.
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.
Thanks for the clarification, please provide the same information for all the files where there is still a global setting.
If it is only one function, why is the setting needed to be global to the file rather than around the constructor?
Out of curiosity where are the version that are shadowed declared?
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.
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.
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.