Silence "redundant move in return statement" warning on GCC9 #18053

1. 
   kristapsk commented at 1:51 AM on February 2, 2020: contributor

   SIlence this warning on GCC9:

     CXX      interfaces/libbitcoin_server_a-chain.o
   interfaces/chain.cpp: In member function ‘virtual std::unique_ptr<interfaces::Chain::Lock> interfaces::{anonymous}::ChainImpl::lock(bool)’:
   interfaces/chain.cpp:243:25: warning: redundant move in return statement [-Wredundant-move]
     243 |         return std::move(result);
         |                ~~~~~~~~~^~~~~~~~
   interfaces/chain.cpp:243:25: note: remove ‘std::move’ call
   

   According to comment above this code, std::move() cannot be removed here, as is needed for some compilers, so silencing using GCC diagnostic pragmas is the only way.

   "-Wpragmas" part is needed because on older compilers, like GCC7, you will have the following warning otherwise:

   interfaces/chain.cpp: In member function 'virtual std::unique_ptr<interfaces::Chain::Lock> interfaces::{anonymous}::ChainImpl::lock(bool)':
   interfaces/chain.cpp:244:32: warning: unknown option after '#pragma GCC diagnostic' kind [-Wpragmas]
    #pragma GCC diagnostic ignored "-Wredundant-move"
                                   ^~~~~~~~~~~~~~~~~~
   

2. fanquake added the label Refactoring on Feb 2, 2020
3. Silence "redundant move in return statement" warning on GCC9 4f797245ec
4. kristapsk force-pushed on Feb 2, 2020
5. kristapsk requested review from ryanofsky on Feb 2, 2020
6. kristapsk requested review from practicalswift on Feb 2, 2020
7. 
   kristapsk commented at 3:07 AM on February 2, 2020: contributor

   Added also checks for GCC, to make MS Visual C++ and potentially other compilers happy.

8. 
   sipa commented at 3:15 AM on February 2, 2020: member

   Wouldn't it be better to just make the std::move conditional on the (broken) compiler version?

9. 
   kristapsk commented at 3:18 AM on February 2, 2020: contributor

   Maybe, but I'm not sure which are the compilers that needs it. Probably @ryanofsky and/or @practicalswift should know, as they have written that comment above.

10. 
    hebasto commented at 8:26 AM on February 2, 2020: member

    More discussion is available in #16995.

11. 
    MarcoFalke commented at 9:56 AM on February 2, 2020: member

    The redundant std::move can probably be removed when we upgrade to C++17. Not sure if it is worth it to over-optimize on compiler warnings until then.

12. 
    fanquake commented at 5:36 AM on February 3, 2020: member

    I agree with Marco and sipa. Adding 8 lines of #ifdefs and #pragmas to quieten this warning seems like overkill.

13. 
    ryanofsky commented at 4:34 PM on February 4, 2020: member

    I don't think 4f797245ecbb5484e798219ab8fe2a2fc729b710 is a good fix because it it is only addressing the issue obliquely with no reference to CWG1579. Also it is verbose as fanquake points out.

    My preferred fix would be the one I suggested previously in this comment: #16995 (comment), which net removes 1 line of code instead of adding 8 lines. I'd be happy if the fix were adopted in this PR.

    Also relevant: #16426 removes this code and will fix the warning

14. 
    kristapsk commented at 7:11 PM on February 4, 2020: contributor

    @ryanofsky , thanks, will look into reviewing #16426.

15. MarcoFalke added the label Waiting for author on Feb 5, 2020
16. 
    laanwj commented at 12:19 PM on February 6, 2020: member

    I could re-open #16995, at least the commits that are still relevant, hadn't noticed the discussion afterward.

17. 
    laanwj commented at 12:44 PM on February 6, 2020: member

    Closing in favor of #16995.

18. laanwj closed this on Feb 6, 2020

    ---

19. ryanofsky removed the label Waiting for author on Feb 24, 2020
20. DrahtBot locked this on Feb 15, 2022

Review Requested
practicalswift