Enable -Wunreachable-code

...motivated by <https://gerrit.libreoffice.org/#/c/41565/2> adding dead code
at the end of a switch statement, after the last case's "break".

-Wunreachable-code appears to work well on Clang, while it appears to have no
effect on GCC.

Most of the affected places are apparently temporary/TODO/FIXME cases of
disabling code via "if (false)", which can be written with an extra set of
parentheses as "if ((false))" to silence -Wunreachable-code on Clang (which thus
needed loplugin:unnecessaryparen to be adapted accordingly).  In some cases,
the controlling expression was more complex than just "false" and needed to be
rewritten by taking it out of the if statement to silence Clang.

One noteworthy case where the nature of the disabled code wasn't immediately
apparent:

  Sep 12 16:59:58 <sberg> quikee, is that "if (false)" in
   ScExponentialSmoothingDialog::ApplyOutput
   (sc/source/ui/StatisticsDialogs/ExponentialSmoothingDialog.cxx) some work-in-
   progress or dead code?
  Sep 12 17:02:03 <quikee> sberg: WIP, but you can remove it
  Sep 12 17:04:47 <sberg> quikee, I'll wrap the false in an extra set of
   parentheses for now, to silence -Wunreachable-code (I wouldn't want to
   remove it, as I have no idea whether I should then also remove the "Initial
   value" comment preceding it)
  Sep 12 17:07:29 <quikee> sberg: both are different ways to calculate the
   "intital value"... so no

Another case where the nature of the dead code, following while (true) loops
without breaks, is unclear is sd/source/ui/remotecontrol/BluetoothServer.cxx,
where I added TODO markers to the workarounds that silence the warnings for now.

basic/source/sbx/sbxvalue.cxx had a variable of type double, of automatic
storage duration, and without an initalizer at the top of a switch statement.
Clang warning about it is arguably a false positive.

Apart from that, this didn't find any cases of genuinely dead code in the
existing code base.

Change-Id: Ib00b822c8efec94278c048783d5997b8ba86a94c
Reviewed-on: https://gerrit.libreoffice.org/42217Tested-by: Stephan Bergmann <sbergman@redhat.com>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

basic/source/sbx/sbxvalue.cxx

@@ -1062,10 +1062,10 @@ bool SbxValue::Compute( SbxOperator eOp, const SbxValue& rOp )
 
                 if( Get( aL ) ) switch( eOp )
                 {
-                    double dTest;
                     case SbxMUL:
+                        {
                             // first overflow check: see if product will fit - test real value of product (hence 2 curr factors)
-                        dTest = (double)aL.nInt64 * (double)aR.nInt64 / (double)CURRENCY_FACTOR_SQUARE;
+                            double dTest = (double)aL.nInt64 * (double)aR.nInt64 / (double)CURRENCY_FACTOR_SQUARE;
                             if( dTest < SbxMINCURR || SbxMAXCURR < dTest)
                             {
                                 aL.nInt64 = SAL_MAX_INT64;
@@ -1084,15 +1084,17 @@ bool SbxValue::Compute( SbxOperator eOp, const SbxValue& rOp )
                             aL.nInt64 *= aR.nInt64;
                             aL.nInt64 /= CURRENCY_FACTOR;
                             break;
+                        }
 
                     case SbxDIV:
+                        {
                             if( !aR.nInt64 )
                             {
                                 SetError( ERRCODE_BASIC_ZERODIV );
                                 break;
                             }
                             // first overflow check: see if quotient will fit - calc real value of quotient (curr factors cancel)
-                        dTest = (double)aL.nInt64 / (double)aR.nInt64;
+                            double dTest = (double)aL.nInt64 / (double)aR.nInt64;
                             if( dTest < SbxMINCURR || SbxMAXCURR < dTest)
                             {
                                 SetError( ERRCODE_BASIC_MATH_OVERFLOW );
@@ -1109,9 +1111,11 @@ bool SbxValue::Compute( SbxOperator eOp, const SbxValue& rOp )
                             aL.nInt64 *= CURRENCY_FACTOR;
                             aL.nInt64 /= aR.nInt64;
                             break;
+                        }
 
                     case SbxPLUS:
-                        dTest = ( (double)aL.nInt64 + (double)aR.nInt64 ) / (double)CURRENCY_FACTOR;
+                        {
+                            double dTest = ( (double)aL.nInt64 + (double)aR.nInt64 ) / (double)CURRENCY_FACTOR;
                             if( dTest < SbxMINCURR || SbxMAXCURR < dTest)
                             {
                                 SetError( ERRCODE_BASIC_MATH_OVERFLOW );
@@ -1119,9 +1123,11 @@ bool SbxValue::Compute( SbxOperator eOp, const SbxValue& rOp )
                             }
                             aL.nInt64 += aR.nInt64;
                             break;
+                        }
 
                     case SbxMINUS:
-                        dTest = ( (double)aL.nInt64 - (double)aR.nInt64 ) / (double)CURRENCY_FACTOR;
+                        {
+                            double dTest = ( (double)aL.nInt64 - (double)aR.nInt64 ) / (double)CURRENCY_FACTOR;
                             if( dTest < SbxMINCURR || SbxMAXCURR < dTest)
                             {
                                 SetError( ERRCODE_BASIC_MATH_OVERFLOW );
@@ -1129,6 +1135,7 @@ bool SbxValue::Compute( SbxOperator eOp, const SbxValue& rOp )
                             }
                             aL.nInt64 -= aR.nInt64;
                             break;
+                        }
                     case SbxNEG:
                         aL.nInt64 = -aL.nInt64;
                         break;

chart2/source/view/main/PropertyMapper.cxx

@@ -82,7 +82,7 @@ void PropertyMapper::getValueMap(
     tPropertyNameMap::const_iterator aEnd( rNameMap.end() );
 
     uno::Reference< beans::XMultiPropertySet > xMultiPropSet(xSourceProp, uno::UNO_QUERY);
-    if(false && xMultiPropSet.is())
+    if((false) && xMultiPropSet.is())
     {
         uno::Sequence< rtl::OUString > aPropSourceNames(rNameMap.size());
         uno::Sequence< rtl::OUString > aPropTargetNames(rNameMap.size());

compilerplugins/clang/test/unnecessaryparen.cxx

@@ -34,6 +34,12 @@ int main()
 
     int v1 = (static_cast<short>(1)) + 1; // expected-error {{unnecessary parentheses around cast [loplugin:unnecessaryparen]}}
     (void)v1;
+
+    // No warnings, used to silence -Wunreachable-code:
+    if ((false)) {
+        return 0;
+    }
+    x = (true) ? 0 : 1;
 };
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

compilerplugins/clang/test/vclwidgets.cxx

@@ -22,7 +22,7 @@ struct Widget : public VclReferenceBase
         Widget* p = mpParent;
         (void)p;
         // test against false+
-        p = true ? mpParent.get() : nullptr;
+        p = (true) ? mpParent.get() : nullptr;
     }
 
     ~Widget() override

compilerplugins/clang/unnecessaryparen.cxx

@@ -224,6 +224,12 @@ void UnnecessaryParen::VisitSomeStmt(const Stmt *parent, const Expr* cond, Strin
     if (parenExpr) {
         if (parenExpr->getLocStart().isMacroID())
             return;
+        // Used to silence -Wunreachable-code:
+        if (isa<CXXBoolLiteralExpr>(parenExpr->getSubExpr())
+            && stmtName == "if")
+        {
+            return;
+        }
         // assignments need extra parentheses or they generate a compiler warning
         auto binaryOp = dyn_cast<BinaryOperator>(parenExpr->getSubExpr());
         if (binaryOp && binaryOp->getOpcode() == BO_Assign)

cppu/qa/test_unotype.cxx

@@ -94,7 +94,7 @@ public:
 
 void Test::testUnoType() {
     // Avoid warnings about unused ~DerivedInterface1/2 (see above):
-    if (false) {
+    if ((false)) {
         DerivedInterface1::dummy(nullptr);
         DerivedInterface2::dummy(nullptr);
     }

cui/source/dialogs/about.cxx

@@ -316,7 +316,9 @@ OUString AboutDialog::GetVersionString()
 
     sVersion += "\n" + Application::GetHWOSConfInfo();
 
-    if (EXTRA_BUILDID[0] != '\0')
+    bool const extra = EXTRA_BUILDID[0] != '\0';
+        // extracted from the 'if' to avoid Clang -Wunreachable-code
+    if (extra)
     {
         sVersion += "\n" EXTRA_BUILDID;
     }

sc/source/ui/StatisticsDialogs/ExponentialSmoothingDialog.cxx

@@ -99,7 +99,7 @@ ScRange ScExponentialSmoothingDialog::ApplyOutput(ScDocShell* pDocShell)
         output.nextRow();
 
         // Initial value
-        if (false)
+        if ((false))
         {
             aTemplate.setTemplate("=AVERAGE(%RANGE%)");
             aTemplate.applyRange("%RANGE%", aCurrentRange);

sd/source/ui/remotecontrol/BluetoothServer.cxx

@@ -1207,6 +1207,9 @@ void SAL_CALL BluetoothServer::run()
                 while (DBUS_DISPATCH_DATA_REMAINS == dbus_connection_get_dispatch_status( pConnection ))
                     dbus_connection_dispatch( pConnection );
             }
+            if ((false)) break;
+                // silence Clang -Wunreachable-code after loop (TODO: proper
+                // fix?)
         }
         unregisterBluez5Profile( pConnection );
         g_main_context_unref( mpImpl->mpContext );
@@ -1295,6 +1298,8 @@ void SAL_CALL BluetoothServer::run()
                 pCommunicator->launch();
             }
         }
+        if ((false)) break;
+            // silence Clang -Wunreachable-code after loop (TODO: proper fix?)
     }
 
     unregisterBluez5Profile( pConnection );

solenv/gbuild/platform/com_GCC_defs.mk

@@ -54,6 +54,7 @@ gb_CFLAGS_COMMON := \
 	-Wextra \
 	-Wstrict-prototypes \
 	-Wundef \
+	-Wunreachable-code \
 	-Wunused-macros \
 	-finput-charset=UTF-8 \
 	-fmessage-length=0 \
@@ -67,6 +68,7 @@ gb_CXXFLAGS_COMMON := \
 	-Wendif-labels \
 	-Wextra \
 	-Wundef \
+	-Wunreachable-code \
 	-Wunused-macros \
 	-finput-charset=UTF-8 \
 	-fmessage-length=0 \

sw/source/core/doc/tblrwcl.cxx

@@ -3985,7 +3985,7 @@ static bool lcl_SetOtherLineHeight( SwTableLine* pLine, CR_SetLineHeight& rParam
             // Calculate the new relative size by means of the old one
             // If the selected Box get bigger, adjust via the max space else
             // via the max height.
-            if( true /*!rParam.bBigger*/ )
+            if( (true) /*!rParam.bBigger*/ )
             {
                 nDist *= pLineFrame->Frame().Height();
                 nDist /= rParam.nMaxHeight;

sw/source/core/undo/undobj1.cxx

@@ -345,7 +345,7 @@ OUString SwUndoInsLayFormat::GetComment() const
     // have a SwDrawContact yet, so it will fall back to SwUndo::GetComment(),
     // which sets pComment to a wrong value.
 //    if (! pComment)
-    if (true)
+    if ((true))
     {
         /*
           If frame format is present and has an SdrObject use the undo

unotools/source/i18n/localedatawrapper.cxx

@@ -1376,7 +1376,7 @@ OUString LocaleDataWrapper::getDate( const Date& rDate ) const
     sal_Int16   nYear   = rDate.GetYear();
     sal_uInt16  nYearLen;
 
-    if ( true /* IsDateCentury() */ )
+    if ( (true) /* IsDateCentury() */ )
         nYearLen = 4;
     else
     {
@@ -1490,7 +1490,7 @@ OUString LocaleDataWrapper::getDuration( const tools::Time& rTime, bool bSec, bo
     if ( rTime < tools::Time( 0 ) )
         pBuf = ImplAddString( pBuf, ' ' );
 
-    if ( true /* IsTimeLeadingZero() */ )
+    if ( (true) /* IsTimeLeadingZero() */ )
         pBuf = ImplAddUNum( pBuf, rTime.GetHour(), 2 );
     else
         pBuf = ImplAddUNum( pBuf, rTime.GetHour() );

unoxml/source/rdf/CLiteral.cxx

@@ -102,7 +102,7 @@ void SAL_CALL CLiteral::initialize(const css::uno::Sequence< css::uno::Any > & a
             "CLiteral::initialize: argument must be string", *this, 0);
     }
     //FIXME: what is legal?
-    if (true) {
+    if ((true)) {
         m_Value = arg0;
     } else {
         throw css::lang::IllegalArgumentException(

unoxml/source/rdf/CURI.cxx

@@ -765,7 +765,7 @@ void SAL_CALL CURI::initialize(const css::uno::Sequence< css::uno::Any > & aArgu
             "CURI::initialize: argument is not valid namespace", *this, 0);
     }
     //FIXME: what is legal?
-    if (true) {
+    if ((true)) {
         m_LocalName = arg1;
     } else {
         throw css::lang::IllegalArgumentException(

vcl/unx/gtk/gtkinst.cxx

@@ -99,7 +99,9 @@ extern "C"
         GtkYieldMutex *pYieldMutex;
 
         // init gdk thread protection
-        if ( !g_thread_supported() )
+        bool const sup = g_thread_supported();
+            // extracted from the 'if' to avoid Clang -Wunreachable-code
+        if ( !sup )
             g_thread_init( nullptr );
 
         gdk_threads_set_lock_functions (GdkThreadsEnter, GdkThreadsLeave);