build: split warnings out of CXXFLAGS#13306
Conversation
CXXFLAGS should not be modified anyway. Also, this will enable us to selectively disable warnings.
|
Concept ack. Could be done with a single var |
|
@Empact They were split so that we may filter out warnings, but still quash annoying ones. |
|
To be clear, I just mean that you could store both the warn and nowarn options in the WARN_CXXFLAGS. No change in options communicated. |
|
Very good idea! Will allow for enabling more warnings Concept ACK |
|
utACK 9e305b5 |
|
Concept ACK. I don't understand autotools enough to see the effect of these code changes. |
As I understand it, there isn't any effect as-is, this just isolates the warnings into a different variable. |
|
@Empact That would not change the result here, but it would mean that we're unable to make a distinction between the two later. @laanwj yes, exactly. @sipa mentioned that he would like to add more aggressive warnings for our code (-Wsuggest-override specifically was the prompt), but that would mean that we would have to fix up a bunch of warnings inside of leveldb. This PR separates the warnings so that they can be removed from leveldb as such: diff --git a/src/Makefile.leveldb.include b/src/Makefile.leveldb.include
index 833f3d2..36edef9 100644
--- a/src/Makefile.leveldb.include
+++ b/src/Makefile.leveldb.include
@@ -30,7 +30,7 @@ LEVELDB_CPPFLAGS_INT += -DLEVELDB_PLATFORM_POSIX
endif
leveldb_libleveldb_a_CPPFLAGS = $(AM_CPPFLAGS) $(LEVELDB_CPPFLAGS_INT) $(LEVELDB_CPPFLAGS)
-leveldb_libleveldb_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
+leveldb_libleveldb_a_CXXFLAGS = $(filter-out $(WARN_CXXFLAGS),$(AM_CXXFLAGS)) $(PIE_FLAGS)
leveldb_libleveldb_a_SOURCES=
leveldb_libleveldb_a_SOURCES += leveldb/port/atomic_pointer.hI didn't include that in this PR because I don't think we want to remove all warnings, so that needs discussion, but this PR is an obvious and straightforward prerequisite IMO. |
|
utACK 9e305b5 |
9e305b5 build: split warnings out of CXXFLAGS (Cory Fields) Pull request description: CXXFLAGS should not be modified anyway. Also, this will enable us to selectively disable warnings. As discussed with @sipa on IRC. Intention is to be able to filter out warnings from leveldb code so that we can be more aggressive with what we enable. Tree-SHA512: 1bf686250f7a59c0aff04371f87c5db4e8f5bde604c6ab75e568326fb6d7733f26b113fa52dc1c836fa10baa76770d479a0e5f82a4a1905947dd7f245e0560f4
9e305b5 build: split warnings out of CXXFLAGS (Cory Fields) Pull request description: CXXFLAGS should not be modified anyway. Also, this will enable us to selectively disable warnings. As discussed with @sipa on IRC. Intention is to be able to filter out warnings from leveldb code so that we can be more aggressive with what we enable. Tree-SHA512: 1bf686250f7a59c0aff04371f87c5db4e8f5bde604c6ab75e568326fb6d7733f26b113fa52dc1c836fa10baa76770d479a0e5f82a4a1905947dd7f245e0560f4
merge bitcoin#18914, bitcoin#13306, bitcoin#16424, bitcoin#13899, bitcoin#17486, bitcoin#17880, bitcoin#18145, bitcoin#18843, bitcoin#16710: split warnings out of CXXFLAGS, add more flags
7 participants
CXXFLAGS should not be modified anyway. Also, this will enable us to selectively disable warnings.
As discussed with @sipa on IRC. Intention is to be able to filter out warnings from leveldb code so that we can be more aggressive with what we enable.