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

#2264: Add stricter warnings for apps that use them #2282

Merged

nlslatt merged 21 commits into develop from 2264-enable-more-warnings

May 28, 2024

Contributor

cz4rs commented May 9, 2024 •

edited

Loading

Enable more warnings when building the project.

The most common warning here is unused variable / parameter. Silencing it with [[maybe_unused]] attribute is explicit and reasonably concise, so I'm going with that in most cases (b832ffc handles cases unsupported in gcc-8).

fixes #2264

github-actions bot commented May 9, 2024 •

edited

Loading

Pipelines results

PR tests (gcc-12, ubuntu, mpich, verbose)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (gcc-8, ubuntu, mpich, address sanitizer)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (clang-10, ubuntu, mpich)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (gcc-10, ubuntu, openmpi, no LB)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (clang-12, ubuntu, mpich)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (clang-13, ubuntu, mpich)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (gcc-9, ubuntu, mpich, zoltan, json schema test)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (intel icpx, ubuntu, mpich, verbose)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (nvidia cuda 11.2, gcc-9, ubuntu, mpich)

Build for 38de7db (2024-05-28 16:40:56 UTC)

/vt/src/vt/pipe/pipe_manager.impl.h(135): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>]"
          detected during:
            instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>]" 
/vt/src/vt/objgroup/proxy/proxy_objgroup.impl.h(204): here
            instantiation of "vt::objgroup::proxy::Proxy<ObjT>::PendingSendType vt::objgroup::proxy::Proxy<ObjT>::reduce<f,Op,Target,Args...>(Target, Args &&...) const [with ObjT=vt::vrt::collection::lb::GreedyLB, f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Op=vt::collective::PlusOp, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>, Args=<vt::vrt::collection::lb::GreedyPayload>]" 
/vt/src/vt/vrt/collection/balance/greedylb/greedylb.cc(222): here

/vt/src/vt/pipe/pipe_manager.impl.h(135): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]" 
/vt/examples/callback/callback.cc(147): here

/vt/src/vt/pipe/pipe_manager.impl.h(135): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]" 
/vt/examples/callback/callback.cc(153): here

/vt/src/vt/pipe/pipe_manager.impl.h(135): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]" 
/vt/examples/callback/callback.cc(147): here

/vt/src/vt/pipe/pipe_manager.impl.h(135): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]" 
/vt/examples/callback/callback.cc(153%0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

PR tests (nvidia cuda 12.2.0, gcc-9, ubuntu, mpich, verbose)

Build for 38de7db (2024-05-28 16:40:56 UTC)

/vt/lib/CLI/CLI/CLI11.hpp(1029): warning #2361-D: invalid narrowing conversion from "double" to "unsigned long"
          TT { std::declval<CC>() }
               ^
          detected during:
            instantiation of "vt::CLI::detail::is_direct_constructible<T, C>::test [with T=std::vector<std::string, std::allocator<std::string>>, C=double]" based on template arguments <std::vector<std::string, std::allocator<std::string>>, double> at line 1041
            instantiation of class "vt::CLI::detail::is_direct_constructible<T, C> [with T=std::vector<std::string, std::allocator<std::string>>, C=double]" at line 5005
            instantiation of "void vt::CLI::Option::results(T &) const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 5034
            instantiation of "T vt::CLI::Option::as<T>() const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 7315

Remark: The warnings can be suppressed with "-diag-suppress <warning-number>"

/vt/lib/CLI/CLI/CLI11.hpp(1029): warning #2361-D: invalid narrowing conversion from "int" to "unsigned long"
          TT { std::declval<CC>() }
               ^
          detected during:
            instantiation of "vt::CLI::detail::is_direct_constructible<T, C>::test [with T=std::vector<std::string, std::allocator<std::string>>, C=int]" based on template arguments <std::vector<std::string, std::allocator<std::string>>, int> at line 1041
            instantiation of class "vt::CLI::detail::is_direct_constructible<T, C> [with T=std::vector<std::string, std::allocator<std::string>>, C=int]" at line 5005
            instantiation of "void vt::CLI::Option::results(T &) const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 5034
            instantiation of "T vt::CLI::Option::as<T>() const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 7315

Testing - passed

PR tests (clang-13, alpine, mpich)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (clang-9, ubuntu, mpich)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (clang-11, ubuntu, mpich)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (clang-14, ubuntu, mpich, verbose)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (gcc-11, ubuntu, mpich, trace runtime, coverage)

Build for 38de7db (2024-05-28 16:40:56 UTC)

Compilation - successful

Testing - passed

PR tests (intel icpc, ubuntu, mpich)

Build for 38de7db (2024-05-28 16:40:56 UTC)

remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get %0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

cz4rs commented

View reviewed changes

src/vt/rdma/channel/rdma_channel.cc Outdated Show resolved Hide resolved

cz4rs force-pushed the 2264-enable-more-warnings branch 2 times, most recently from e3eee52 to 9b1d445 Compare

May 14, 2024 17:27

cz4rs marked this pull request as ready for review

May 14, 2024 18:11

cz4rs force-pushed the 2264-enable-more-warnings branch 8 times, most recently from d56154c to b832ffc Compare

May 16, 2024 15:17

cz4rs commented

View reviewed changes

src/vt/collective/barrier/barrier.cc Outdated Show resolved Hide resolved

cz4rs requested review from stmcgovern, lifflander, nmm0, thearusable, JacobDomagala, ppebay and nlslatt

May 17, 2024 10:12

JacobDomagala approved these changes

View reviewed changes

Contributor

JacobDomagala left a comment

🚀

This was referenced May 17, 2024

Remove support for gcc-8 #2286

Closed

Fix Codacy issues #2287

Open

thearusable approved these changes

View reviewed changes

src/vt/rdma/channel/rdma_channel.cc Outdated Show resolved Hide resolved

cz4rs commented

View reviewed changes

src/vt/rdma/channel/rdma_channel.cc Outdated Show resolved Hide resolved

lifflander approved these changes

View reviewed changes

Collaborator

lifflander left a comment

Looks great!

Collaborator

lifflander commented May 21, 2024

Looks great!

Ready to merge?

Contributor Author

cz4rs commented May 21, 2024

Looks great!

Ready to merge?

Yes!


          #2264: cmake: enable more warnings

70bd070

Enable more warnings and sort the flags alphabetically.

cz4rs added 20 commits

May 28, 2024 18:40


          #2264: fix non-virtual-dtor and suggest-override warnings

2e47128


          #2264: cmake: remove flags for unsupported compilers

efff617


          #2264: remove unused variable

adfa0a7


          #2264: add FIXME for warning

9f1276f


          #2264: remove unused variable


          #2264: use const reference when possible

da8c034


          #2264: scripts: remove unused variables

7292fd5


          #2264: use range-based loop

3adcbb8


          #2264: simplify if logic

f145881


          #2264: cmake: enable -Wextra

de030b9


          #2264: fix unused variable warnings

39c0e47


          #2264: fix remaining warnings (gcc)

c094d39


          #2264: fix unused variable warnings

d8b6904


          #2264: remove unused parameters

72317d9


          #2264: fix unused variable warnings

ea50fd8


          #2264: fix unused variable warnings (cuda, hip)

d7735fa


          #2264: work around gcc-8 limitations

70cbc30


          #2264: remove unnecessary #include

d141682


          #2264: simplify ternary expression

13c5b76


          #2264: remove unused parameter

38de7db

cz4rs force-pushed the 2264-enable-more-warnings branch from e1e0155 to 38de7db Compare

May 28, 2024 16:41

nlslatt merged commit faba6a5 into develop

26 checks passed

Member

PhilMiller commented May 29, 2024

Looking ahead, you may want to explicitly add warnings about performance-impacting things -Wredundant-move, -Wpessimizing-move, and -Wrange-loop-construct, separate from their current default inclusion in GCC's -Wall

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JacobDomagala JacobDomagala approved these changes

thearusable thearusable approved these changes

lifflander lifflander approved these changes

stmcgovern Awaiting requested review from stmcgovern

nmm0 Awaiting requested review from nmm0

ppebay Awaiting requested review from ppebay

nlslatt Awaiting requested review from nlslatt

Labels

None yet