Skip to content

build: Disable Wshadow warning #10136

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 2, 2017
Merged

build: Disable Wshadow warning #10136

merged 1 commit into from
Apr 2, 2017

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Apr 1, 2017 •
edited
Loading

This warning was enabled by default in #8808 but it's a continuing source of annoyance for me and other developers. I'm sick of sounding like a broken record, so disable it again.

@laanwj
build: Disable Wshadow warning
2c83911 
This warning was enabled by default in bitcoin#8808 but it's a
[continuing](bitcoin#9911 (comment))
[source](bitcoin#10089 (comment)) of
[annoyance](bitcoin#9911 (comment)) for me
and other developers. I'm sick of sounding like a broken record, so disable it again.
@laanwj laanwj mentioned this pull request Apr 1, 2017
Merged
@jnewbery
Copy link
Contributor

jnewbery commented Apr 1, 2017

concept ACK (with apologies to @paveljanik!)

If we can find a more intelligent way of managing these warnings so they're not as annoying, I'm all for it. At the moment it feels like the Wshadow tail is wagging the dog.

@maflcko
Copy link
Member

maflcko commented Apr 1, 2017 via email

@jonasschnelli
Copy link
Contributor

utACK 2c83911

@laanwj
Copy link
Member Author

laanwj commented Apr 2, 2017 •
edited
Loading

If we can find a more intelligent way of managing these warnings so they're not as annoying, I'm all for it. At the moment it feels like the Wshadow tail is wagging the dog.

This was clearly the wrong way to prevent variable name shadows.

Agreed. And there is still value in compiling with Wshadow periodically (with a carefully chosen compiler) and seeing if it finds any bug.

@laanwj laanwj merged commit 2c83911 into bitcoin:master Apr 2, 2017
laanwj added a commit that referenced this pull request Apr 2, 2017
@laanwj
Merge #10136: build: Disable Wshadow warning
81da4c7 
2c83911 build: Disable Wshadow warning (Wladimir J. van der Laan)

Tree-SHA512: e3c1f7253c43449740760da287985b8027344dfc48c8a85ea9bca977c73cbaf75709d6e32ac0fea51eb89dccb48706a5abdf006be45375838df10ccba35e9aa1
@paveljanik
Copy link
Contributor

Postmerge ACK. The real problem is that people use ancient compilers and I have not tested them all (only few selected by some distros as their default compilers).

FWIW: I'll compile all my builds with -Wshadow to catch all new occurrences. And of course, thanks for patience :-)

@maflcko maflcko mentioned this pull request Sep 9, 2018
Closed
@laanwj laanwj mentioned this pull request Feb 10, 2019
Closed
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request May 10, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#10136: build: Disable Wshadow warning
137e860 
2c83911 build: Disable Wshadow warning (Wladimir J. van der Laan)

Tree-SHA512: e3c1f7253c43449740760da287985b8027344dfc48c8a85ea9bca977c73cbaf75709d6e32ac0fea51eb89dccb48706a5abdf006be45375838df10ccba35e9aa1
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request May 15, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#10136: build: Disable Wshadow warning
7d6f8ed 
2c83911 build: Disable Wshadow warning (Wladimir J. van der Laan)

Tree-SHA512: e3c1f7253c43449740760da287985b8027344dfc48c8a85ea9bca977c73cbaf75709d6e32ac0fea51eb89dccb48706a5abdf006be45375838df10ccba35e9aa1
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request May 20, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#10136: build: Disable Wshadow warning
07ed44d 
2c83911 build: Disable Wshadow warning (Wladimir J. van der Laan)

Tree-SHA512: e3c1f7253c43449740760da287985b8027344dfc48c8a85ea9bca977c73cbaf75709d6e32ac0fea51eb89dccb48706a5abdf006be45375838df10ccba35e9aa1
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request May 21, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#10136: build: Disable Wshadow warning
12989cf 
2c83911 build: Disable Wshadow warning (Wladimir J. van der Laan)

Tree-SHA512: e3c1f7253c43449740760da287985b8027344dfc48c8a85ea9bca977c73cbaf75709d6e32ac0fea51eb89dccb48706a5abdf006be45375838df10ccba35e9aa1
barrystyle pushed a commit to PACGlobalOfficial/PAC that referenced this pull request Jan 22, 2020
@laanwj
Merge bitcoin#10136: build: Disable Wshadow warning
cdb5dc2 
2c83911 build: Disable Wshadow warning (Wladimir J. van der Laan)

Tree-SHA512: e3c1f7253c43449740760da287985b8027344dfc48c8a85ea9bca977c73cbaf75709d6e32ac0fea51eb89dccb48706a5abdf006be45375838df10ccba35e9aa1
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Build system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@laanwj @jnewbery @maflcko @jonasschnelli @paveljanik