catchorg / Clara

A simple to use, composable, command line parser for C++ 11 and beyond
Boost Software License 1.0
649 stars 67 forks source link

Fix VS2015 warning C4702: unreachable code. #5

Closed mloskot closed 8 years ago

mloskot commented 8 years ago

This clears warning C4702: unreachable code reported while building Clara-based project (e.g. Catch), in Release configuration, using VS2015 Update 1 with warning level /W4.

Apparently, convertInto invocation resolves to call the always throwing variant:

        template<typename T>
        inline void convertInto( bool, T& ) {
            throw std::runtime_error( "Invalid conversion" );
        }

The issue has been discovered while switching nanodbc project to Catch (lexicalunit/nanodbc#90).

philsquared commented 8 years ago

Thanks for this. Odd that I'm not seeing this (I've been porting a large project - complete with Catch tests - to VS2015 Update 1 recently - and we compile at at least /W4). I'll double check our settings.

In the meantime the way I usually deal with "unreachable code" warnings is to put the part that makes it unreachable (usually a throw, as in this case) inside an if statement, where the if tests the result of isTrue( true ). There isn't currently an isTrue() function in Clara, but it's trivial to add. If this makes sense to you would you mind trying it yourself to verify it addresses your issue (without the need for the #pragmas)? Then either propose a new PR, or just let me know and I'll make the change here.

mloskot commented 8 years ago

@philsquared I have just applied the suggested workaround, but it doesn't seem to help. I still get the warning while building nanodbc with Catch-based tests:

1>------ Build started: Project: odbc_tests, Configuration: Release x64 ------
1>  odbc_test.cpp
1>d:\dev\nanodbc\build64\catch\src\catch\include\external\clara.h(381): error C2220: warning treated as error - no 'object' file generated
1>d:\dev\nanodbc\build64\catch\src\catch\include\external\clara.h(381): warning C4702: unreachable code
1>d:\dev\nanodbc\build64\catch\src\catch\include\external\clara.h(381): warning C4702: unreachable code
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

Perhaps, VS2015 Update 1 has become clever enough to conclude the isTrue is constant condition too.

philsquared commented 8 years ago

Ok, thanks for trying (and letting me know). That's a shame. I noticed last week that our VS2015 project wasn't compiling at /W4 after all - but even when I did set it I still didn't see this particular issue. It occurs to me that there may be some interaction with the optimiser in this case. I don't recall if I tried a release build. So I'll have another look sometime this week.

philsquared commented 8 years ago

I made another tweak in 0.0.2.0 that might help with this. Please let me know, if you get a chance, if it makes a difference for you.

philsquared commented 8 years ago

I've now removed the source of this problem so I'm going to close this PR.

mloskot commented 8 years ago

@philsquared The warning is gone for me in version 0.0.2.2. Thanks!