Enable warnings as error on Linux (#85)

bhardwajs · web-flow · commit 977fa36c7af3 · 2022-03-30T15:54:14.000-07:00
This commit increases warning level and enables warnings as error on
Linux build. Doing this pointed out several cases of sign mismatches,
variable defintion ordering issues, and unused variables.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -65,6 +65,10 @@ else()
       -fvisibility=hidden # By default, hide symbols on ELF binaries
       -g # add debug symbols for build pdb
       -pedantic
+      -Wall
+      -Werror
+      $<$<CXX_COMPILER_ID:GNU>:-Wno-unused-but-set-variable>
+      -Wno-unused-variable
   )
 
   # AppleClang is not accepting -flto as linker option
diff --git a/api/combination.cpp b/api/combination.cpp
@@ -112,7 +112,8 @@ ComboStatus Combination::Feasible( int value )
     ComboStatus retval = ( COVERED == m_bitvec[ value ] ) ? ComboStatus::CoveredMatch : ComboStatus::Open;
     for( ParamCollection::reverse_iterator iter = m_params.rbegin(); iter != m_params.rend(); ++iter )
     {
-        if( ( *iter )->GetBoundCount() && ( *iter )->GetLast() != value % ( *iter )->GetValueCount() )
+        if( ( *iter )->GetBoundCount() && 
+            static_cast<int>(( *iter )->GetLast()) != value % ( *iter )->GetValueCount() )
         {
             return ComboStatus::Excluded;
         }
@@ -143,7 +144,7 @@ int Combination::Weight( int value )
 //
 int Combination::AddBinding()
 {
-    if( ++m_boundCount == m_params.size() )
+    if( ++m_boundCount == static_cast<int>(m_params.size()) )
     {
         // compute which zero we're setting
         size_t value = 0;
@@ -261,7 +262,7 @@ void Combination::SetOpen( int index )
 //
 //
 Combination::Combination( Model *M ) :
-    m_bitvec( nullptr ), m_openCount( 0 ), m_boundCount( 0 ), m_range( 0 ), m_model( M )
+    m_bitvec( nullptr ), m_range( 0 ), m_openCount( 0 ), m_boundCount( 0 ), m_model( M )
 {
     m_id = ++m_lastUsedId;
     DOUT( L"Combination created: " << m_id << endl );
diff --git a/api/generator.h b/api/generator.h
@@ -163,15 +163,13 @@ class GenerationError
 {
 public:
 
-    GenerationError( std::string file, int line, ErrorType err = ErrorType::Unknown ) :
-        _file( file ), _line( line ), _err( err ){}
+    GenerationError( std::string, int, ErrorType err = ErrorType::Unknown ) :
+        _err( err ){}
 
     ErrorType GetErrorType() { return (ErrorType) _err; }
 
 private:
     ErrorType    _err;
-    std::string  _file;
-    int          _line;
 };
 
 //
@@ -316,7 +314,7 @@ class Combination
     int  Bind( int val, WorkList& worklist );
     int  AddBinding();
     int  GetBoundCount() const { return m_boundCount; }
-    bool IsFullyBound()  const { return m_boundCount == m_params.size(); }
+    bool IsFullyBound()  const { return m_boundCount == static_cast<int>(m_params.size()); }
 
     void        SetOpen   ( int n );
     bool        IsOpen    ( int n ) const { return OPEN     == m_bitvec[ n ]; }
@@ -375,9 +373,9 @@ class Parameter
 {
 public:
     Parameter( int order, int sequence, int valueCount, std::wstring name, bool expectedResultParam ) :
-        m_order( order ), m_sequence( sequence ), m_valueCount( valueCount ),
-        m_name( name ), m_expResultParam( expectedResultParam ), m_valueWeights( 0 ),
-        m_bound( false ), m_pending( false ), m_avgExclusionSize( 0 )
+        m_name( name ), m_order( order ), m_sequence( sequence ), m_valueCount( valueCount ),
+        m_expResultParam( expectedResultParam ), m_bound( false ),
+        m_pending( false ), m_valueWeights( 0 ), m_avgExclusionSize( 0 )
     {
          // result params must have order = 1
         if ( m_expResultParam ) m_order = 1;
@@ -503,7 +501,7 @@ class Model
 {
 public:
     Model( const std::wstring& id, GenerationType type, int order, long seed = 0 ) :
-        m_id( id ), m_generationType( type ), m_order( order ), m_maxRows( 0 ), m_lastParamId( UNDEFINED_ID + 1000 * 1000 ) {
+        m_id( id ), m_order( order ), m_maxRows( 0 ), m_generationType( type ), m_lastParamId( UNDEFINED_ID + 1000 * 1000 ) {
         SetRandomSeed( seed );
     }
     ~Model();
@@ -613,7 +611,6 @@ class Model
 
     GenerationType m_generationType;
 
-    Parameter*   m_currentParam;
     unsigned int m_lastParamId;
     long         m_totalCombinations;
     long         m_remainingCombinations;
diff --git a/api/model.cpp b/api/model.cpp
@@ -469,7 +469,7 @@ void Model::markUndefinedValuesInResultParams()
             {
                 // TODO: when result params are suported check modl/modl021.txt doesn't break here
                 if( !i_term->first->IsExpectedResultParam()
-                &&  i_row->at( i_term->first->GetSequence() ) != i_term->second )
+                &&  static_cast<int>(i_row->at( i_term->first->GetSequence() )) != i_term->second )
                 {
                     matches = false;
                     break;
@@ -520,7 +520,7 @@ void Model::markUndefinedValuesInResultParams()
         for( size_t row = 0; row < ip->second.size(); ++row )
         {
             // all values have to be excluded except for 1
-            if( ip->second[ row ].size() != ip->first->GetValueCount() - 1 )
+            if( static_cast<int>(ip->second[ row ].size()) != ip->first->GetValueCount() - 1 )
             {
                 m_results[ row ][ col ] = Parameter::UndefinedValue;
             }
@@ -1061,7 +1061,7 @@ bool Model::mapExclusionsToPseudoParameters()
                                 break;
                             }
                             int nParam = static_cast<int>( distance( comps->begin(), ip ) );
-                            size_t nRealVal = param->GetModel()->GetResults()[ vidx ][ nParam ];
+                            int nRealVal = static_cast<int>(param->GetModel()->GetResults()[ vidx ][ nParam ]);
                             if( nRealVal != irel->second )
                             {
                                 break;
@@ -1227,7 +1227,7 @@ bool Model::rowViolatesExclusion( ResultRow& row )
         bool matches = true;
         for( Exclusion::iterator it = ie->begin(); it != ie->end(); ++it )
         {
-            if( row[ it->first->GetSequence() ] != it->second )
+            if( static_cast<int>(row[ it->first->GetSequence() ]) != it->second )
             {
                 matches = false;
                 break;
@@ -1302,7 +1302,7 @@ void Model::mapRowSeedsToPseudoParameters()
                                 break;
                             }
                             int nParam = static_cast<int>( distance( comps->begin(), ip ) );
-                            size_t nRealVal = param->GetModel()->GetResults()[ vidx ][ nParam ];
+                            int nRealVal = static_cast<int>(param->GetModel()->GetResults()[ vidx ][ nParam ]);
                             if( nRealVal != irel->second )
                             {
                                 break;
diff --git a/api/trie.h b/api/trie.h
@@ -76,7 +76,7 @@ class trie
                 {
                     node = new trienode < typename Col::value_type >;
                 }
-                catch( std::bad_alloc e )
+                catch(const std::bad_alloc& )
                 {
                     return( false );
                 }
diff --git a/cli/ccommon.h b/cli/ccommon.h
@@ -153,15 +153,15 @@ class CTerm
         IN std::wstring rawText    // raw text of the term, useful for warnings
         ) :
             Parameter    ( parameter ),
-            RelationType ( relationType ),
             DataType     ( dataType ),
-            Data         ( data ),
-            RawText      ( rawText ) {}
+            RelationType ( relationType ),
+            RawText      ( rawText ),
+            Data         ( data ) {}
 
     CTerm( CTerm& Term ) :
         Parameter   ( Term.Parameter ),
-        RelationType( Term.RelationType ),
         DataType    ( Term.DataType ),
+        RelationType( Term.RelationType ),
         RawText     ( Term.RawText )
     {
         assert( RelationType < RelationType::Unknown );
@@ -215,8 +215,8 @@ class CTerm
     CParameter*                       Parameter;
     TermDataType                      DataType;
     pictcli_constraints::RelationType RelationType;
-    void*                             Data;
     std::wstring                      RawText;
+    void*                             Data;
 };
 
 //
diff --git a/cli/gcdexcl.cpp b/cli/gcdexcl.cpp
@@ -642,7 +642,7 @@ bool ConstraintsInterpreter::ConvertToExclusions( OUT CGcdExclusions& gcdExclusi
         // last-minute cleanup
         removeContradictingExclusions( gcdExclusions );
     }
-    catch( std::bad_alloc e )
+    catch( const std::bad_alloc& )
     { 
         throw new GenerationError( __FILE__, __LINE__, ErrorType::OutOfMemory );
     }
diff --git a/cli/gcdmodel.cpp b/cli/gcdmodel.cpp
@@ -114,7 +114,7 @@ bool CGcdData::FixParamOrder( IN Model* submodel )
             {
                 auto p = _modelData.FindParameterByGcdPointer( param );
                 assert( p != _modelData.Parameters.end() );
-                if( p->Order != UNDEFINED_ORDER )
+                if( p->Order != static_cast<unsigned int>(UNDEFINED_ORDER) )
                 {
                     // TODO: add verification of Order
                     // if p->Order > model->parameters.count - model.ResultParameters.count then error out
diff --git a/cli/model.h b/cli/model.h
@@ -23,8 +23,8 @@ class CModelValue
         unsigned int weight,
         bool         positive
         ) : _names           ( names ),
-            _weight          ( weight ),
             _positive        ( positive ),
+            _weight          ( weight ),
             _currentNameIndex( 0 ) {}
 
     wstrings& GetAllNames()       { return( _names ); }
@@ -60,8 +60,8 @@ class CModelParameter
 
     CModelParameter() :
         Name(L""),
-        IsResultParameter(false),
         Order(static_cast<unsigned int>(UNDEFINED_ORDER)),
+        IsResultParameter(false),
         GcdPointer(nullptr){}
 
     int GetValueOrdinal( IN std::wstring& name, IN bool caseSensitive );
@@ -123,9 +123,9 @@ class CModelData
         CaseSensitive(false),
         Verbose(false),
         Statistics(false),
-        RowSeedsFile(L""),
         GenerationMode(GenerationMode::Regular),
         MaxApproxTries(1000),
+        RowSeedsFile(L""),
         ConstraintPredicates(L""),
         m_hasNegativeValues(false),
         m_encoding(EncodingType::ANSI),
diff --git a/cli/pict.cpp b/cli/pict.cpp
@@ -137,7 +137,7 @@ int main
     {
         loc = std::locale("C.UTF-8");
     }
-    catch ( std::runtime_error )
+    catch ( const std::runtime_error&)
     {
         loc = std::locale::classic();
     }

Original file line number	Diff line number	Diff line change
`@@ -112,7 +112,8 @@ ComboStatus Combination::Feasible( int value )`
`112`	`112`	`ComboStatus retval = ( COVERED == m_bitvec[ value ] ) ? ComboStatus::CoveredMatch : ComboStatus::Open;`
`113`	`113`	`for( ParamCollection::reverse_iterator iter = m_params.rbegin(); iter != m_params.rend(); ++iter )`
`114`	`114`	`{`
`115`		`- if( ( iter )->GetBoundCount() && ( iter )->GetLast() != value % ( *iter )->GetValueCount() )`
	`115`	`+ if( ( *iter )->GetBoundCount() &&`
	`116`	`+ static_cast<int>(( iter )->GetLast()) != value % ( iter )->GetValueCount() )`
`116`	`117`	`{`
`117`	`118`	`return ComboStatus::Excluded;`
`118`	`119`	`}`
`@@ -143,7 +144,7 @@ int Combination::Weight( int value )`
`143`	`144`	`//`
`144`	`145`	`int Combination::AddBinding()`
`145`	`146`	`{`
`146`		`- if( ++m_boundCount == m_params.size() )`
	`147`	`+ if( ++m_boundCount == static_cast<int>(m_params.size()) )`
`147`	`148`	`{`
`148`	`149`	`// compute which zero we're setting`
`149`	`150`	`size_t value = 0;`
`@@ -261,7 +262,7 @@ void Combination::SetOpen( int index )`
`261`	`262`	`//`
`262`	`263`	`//`
`263`	`264`	`Combination::Combination( Model *M ) :`
`264`		`- m_bitvec( nullptr ), m_openCount( 0 ), m_boundCount( 0 ), m_range( 0 ), m_model( M )`
	`265`	`+ m_bitvec( nullptr ), m_range( 0 ), m_openCount( 0 ), m_boundCount( 0 ), m_model( M )`
`265`	`266`	`{`
`266`	`267`	`m_id = ++m_lastUsedId;`
`267`	`268`	`DOUT( L"Combination created: " << m_id << endl );`
Original file line number	Diff line number	Diff line change
`@@ -469,7 +469,7 @@ void Model::markUndefinedValuesInResultParams()`
`469`	`469`	`{`
`470`	`470`	`// TODO: when result params are suported check modl/modl021.txt doesn't break here`
`471`	`471`	`if( !i_term->first->IsExpectedResultParam()`
`472`		`- && i_row->at( i_term->first->GetSequence() ) != i_term->second )`
	`472`	`+ && static_cast<int>(i_row->at( i_term->first->GetSequence() )) != i_term->second )`
`473`	`473`	`{`
`474`	`474`	`matches = false;`
`475`	`475`	`break;`
`@@ -520,7 +520,7 @@ void Model::markUndefinedValuesInResultParams()`
`520`	`520`	`for( size_t row = 0; row < ip->second.size(); ++row )`
`521`	`521`	`{`
`522`	`522`	`// all values have to be excluded except for 1`
`523`		`- if( ip->second[ row ].size() != ip->first->GetValueCount() - 1 )`
	`523`	`+ if( static_cast<int>(ip->second[ row ].size()) != ip->first->GetValueCount() - 1 )`
`524`	`524`	`{`
`525`	`525`	`m_results[ row ][ col ] = Parameter::UndefinedValue;`
`526`	`526`	`}`
`@@ -1061,7 +1061,7 @@ bool Model::mapExclusionsToPseudoParameters()`
`1061`	`1061`	`break;`
`1062`	`1062`	`}`
`1063`	`1063`	`int nParam = static_cast<int>( distance( comps->begin(), ip ) );`
`1064`		`- size_t nRealVal = param->GetModel()->GetResults()[ vidx ][ nParam ];`
	`1064`	`+ int nRealVal = static_cast<int>(param->GetModel()->GetResults()[ vidx ][ nParam ]);`
`1065`	`1065`	`if( nRealVal != irel->second )`
`1066`	`1066`	`{`
`1067`	`1067`	`break;`
`@@ -1227,7 +1227,7 @@ bool Model::rowViolatesExclusion( ResultRow& row )`
`1227`	`1227`	`bool matches = true;`
`1228`	`1228`	`for( Exclusion::iterator it = ie->begin(); it != ie->end(); ++it )`
`1229`	`1229`	`{`
`1230`		`- if( row[ it->first->GetSequence() ] != it->second )`
	`1230`	`+ if( static_cast<int>(row[ it->first->GetSequence() ]) != it->second )`
`1231`	`1231`	`{`
`1232`	`1232`	`matches = false;`
`1233`	`1233`	`break;`
`@@ -1302,7 +1302,7 @@ void Model::mapRowSeedsToPseudoParameters()`
`1302`	`1302`	`break;`
`1303`	`1303`	`}`
`1304`	`1304`	`int nParam = static_cast<int>( distance( comps->begin(), ip ) );`
`1305`		`- size_t nRealVal = param->GetModel()->GetResults()[ vidx ][ nParam ];`
	`1305`	`+ int nRealVal = static_cast<int>(param->GetModel()->GetResults()[ vidx ][ nParam ]);`
`1306`	`1306`	`if( nRealVal != irel->second )`
`1307`	`1307`	`{`
`1308`	`1308`	`break;`
Original file line number	Diff line number	Diff line change
`@@ -76,7 +76,7 @@ class trie`
`76`	`76`	`{`
`77`	`77`	`node = new trienode < typename Col::value_type >;`
`78`	`78`	`}`
`79`		`- catch( std::bad_alloc e )`
	`79`	`+ catch(const std::bad_alloc& )`
`80`	`80`	`{`
`81`	`81`	`return( false );`
`82`	`82`	`}`
Original file line number	Diff line number	Diff line change
`@@ -642,7 +642,7 @@ bool ConstraintsInterpreter::ConvertToExclusions( OUT CGcdExclusions& gcdExclusi`
`642`	`642`	`// last-minute cleanup`
`643`	`643`	`removeContradictingExclusions( gcdExclusions );`
`644`	`644`	`}`
`645`		`- catch( std::bad_alloc e )`
	`645`	`+ catch( const std::bad_alloc& )`
`646`	`646`	`{`
`647`	`647`	`throw new GenerationError( __FILE__, __LINE__, ErrorType::OutOfMemory );`
`648`	`648`	`}`
Original file line number	Diff line number	Diff line change
`@@ -114,7 +114,7 @@ bool CGcdData::FixParamOrder( IN Model* submodel )`
`114`	`114`	`{`
`115`	`115`	`auto p = _modelData.FindParameterByGcdPointer( param );`
`116`	`116`	`assert( p != _modelData.Parameters.end() );`
`117`		`- if( p->Order != UNDEFINED_ORDER )`
	`117`	`+ if( p->Order != static_cast<unsigned int>(UNDEFINED_ORDER) )`
`118`	`118`	`{`
`119`	`119`	`// TODO: add verification of Order`
`120`	`120`	`// if p->Order > model->parameters.count - model.ResultParameters.count then error out`
Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,7 @@ int main`
`137`	`137`	`{`
`138`	`138`	`loc = std::locale("C.UTF-8");`
`139`	`139`	`}`
`140`		`- catch ( std::runtime_error )`
	`140`	`+ catch ( const std::runtime_error&)`
`141`	`141`	`{`
`142`	`142`	`loc = std::locale::classic();`
`143`	`143`	`}`