unit-e
unit-e copied to clipboard
dtr-org
-Werror for builds
Here are some clang/macOS warnings that should be tackled along with this:
CXX libunite_server_a-init.o
In file included from init.cpp:10:
In file included from ./init.h:9:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/string:477:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/string_view:176:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/__string:56:
In file included from /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/algorithm:643:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:2285:5: warning: delete called on non-final 'PeerLogicValidation' that has virtual functions but non-virtual destructor
[-Wdelete-non-virtual-dtor]
delete __ptr;
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:2598:7: note: in instantiation of member function 'std::__1::default_delete<PeerLogicValidation>::operator()' requested here
__ptr_.second()(__tmp);
^
init.cpp:211:15: note: in instantiation of member function 'std::__1::unique_ptr<PeerLogicValidation, std::__1::default_delete<PeerLogicValidation> >::reset' requested here
peerLogic.reset();
^
1 warning generated.
CXX leveldb/util/libleveldb_a-logging.o
leveldb/util/logging.cc:58:40: warning: comparison of integers of different signs: 'const int' and 'unsigned long long' [-Wsign-compare]
(v == kMaxUint64/10 && delta > kMaxUint64%10)) {
~~~~~ ^ ~~~~~~~~~~~~~
1 warning generated.
CXX unite_cli-unite-cli.o
In file included from unite-cli.cpp:10:
In file included from ./chainparamsbase.h:8:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:2285:5: warning: delete called on 'BaseRequestHandler' that is abstract but has non-virtual destructor
[-Wdelete-non-virtual-dtor]
delete __ptr;
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:2598:7: note: in instantiation of member function 'std::__1::default_delete<BaseRequestHandler>::operator()' requested here
__ptr_.second()(__tmp);
^
unite-cli.cpp:409:16: note: in instantiation of member function 'std::__1::unique_ptr<BaseRequestHandler, std::__1::default_delete<BaseRequestHandler> >::reset' requested here
rh.reset(new GetinfoRequestHandler());
^
1 warning generated.
CXX test/test_unite-test_unite.o
In file included from test/test_unite.cpp:5:
In file included from ./test/test_unite.h:8:
In file included from ./chainparamsbase.h:8:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:2285:5: warning: delete called on non-final 'PeerLogicValidation' that has virtual functions but non-virtual destructor
[-Wdelete-non-virtual-dtor]
delete __ptr;
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/memory:2598:7: note: in instantiation of member function 'std::__1::default_delete<PeerLogicValidation>::operator()' requested here
__ptr_.second()(__tmp);
^
test/test_unite.cpp:123:19: note: in instantiation of member function 'std::__1::unique_ptr<PeerLogicValidation, std::__1::default_delete<PeerLogicValidation> >::reset' requested here
peerLogic.reset(new PeerLogicValidation(connman, scheduler));
^
1 warning generated.
@kostyantyn started work on this in https://github.com/dtr-org/unit-e/tree/limit_number_of_warnings
Do we want to add -Wall -pedantic as well? Personally, I like these flags but want to know what others think about it.
@frolosofsky I think it makes sense. BTW, I am not currently working on it. If someone has time, feel free to pick it up and assign yourself. Otherwise, will leave a comment when I can resume the work.
@kostyantyn @cornelius In bitcoin there's an "up for grabs" label for these cases. I think it suffices if you remove yourself from it as an assignee.
Do we want to add -Wall -pedantic as well? Personally, I like these flags but want to know what others think about it.
The question is how many warnings will pop out of it if we enable these and how much bitcoin do we have to change for this.
We tried to make all warnings an error (also enabled more warnings with -pedantic) but we got too many errors (especially coming from 3rd party libraries).
Would be good if we could compile different targets with the different flags (maybe we would need to introduce more targets). Then we can be stricter in some "sub-folders".
And to make this feature roll, we can turn one warning as an error and keep adding them instead of excluding from the list.
Would be good if we could compile different targets with the different flags (maybe we would need to introduce more targets). Then we can be stricter in some "sub-folders".
Yes, you can add different options for different cpp/object files. But, if the header contains a warning, we will need to disable a warning for every cpp file that includes this header. In fact, it could make such change meaningless. So that, it’d be great actually to fix all the warnings.
If we tackle this I think it might sense to do it along/after https://github.com/dtr-org/unit-e/issues/61 and any way it should be backported to bitcoin. If we plan to merge with forthcoming versions we will end up fixing a lot of stuff over and over again otherwise.
If we tackle this I think it might sense to do it along/after #61 and any way it should be backported to bitcoin. If we plan to merge with forthcoming versions we will end up fixing a lot of stuff over and over again otherwise.
Both issues look like great candidates to be addressed between the Testnet (0.1) and the Mainnet (1.0) milestones
On this topic: http://fastcompression.blogspot.com/2019/01/compiler-warnings.html