-
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?
Changes from 3 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
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 |
|---|---|---|
| @@ -1,3 +1,7 @@ | ||
| #if defined(__clang__) | ||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wshadow" | ||
| #endif | ||
| // @(#)root/base:$Id$ | ||
| // Author: Rene Brun 12/05/95 | ||
|
|
||
|
|
@@ -53,7 +57,14 @@ class TAttMarker { | |
| ClassDef(TAttMarker,3); //Marker attributes | ||
| }; | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic push | ||
| #pragma clang diagnostic ignored "-Wshadow" | ||
| #endif | ||
| enum EMarkerStyle {kDot=1, kPlus, kStar, kCircle=4, kMultiply=5, | ||
|
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 document exactly which of the enums constant are the one shadowing existing symbols and list/refer where the shadowed symbols comes from (if known).
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 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
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. 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?
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 point was to activate the suppression for the entire class scope. Since the compiler processes the file sequentially, the directive must be placed before the inline constructor definition. Placing it at the start of the class guarantees the constructor is covered; the enum just happens to be the first declaration in that scope. |
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #endif | ||
| kFullDotSmall=6, kFullDotMedium=7, kFullDotLarge=8, | ||
| kFullCircle=20, kFullSquare=21, kFullTriangleUp=22, | ||
| kFullTriangleDown=23, kOpenCircle=24, kOpenSquare=25, | ||
|
|
@@ -70,3 +81,6 @@ class TAttMarker { | |
|
|
||
| #endif | ||
|
|
||
| #if defined(__clang__) | ||
| #pragma clang diagnostic pop | ||
| #endif | ||
Uh oh!
There was an error while loading. Please reload this page.