Enable warnings as error on Linux (#85)

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.
2022-03-30 15:54:14 -07:00 · 2022-03-30 15:54:14 -07:00 · 977fa36c7a
--- 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
--- 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 );
--- 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 @@ public:
    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 @@ private:

    GenerationType m_generationType;

-    Parameter*   m_currentParam;
    unsigned int m_lastParamId;
    long         m_totalCombinations;
    long         m_remainingCombinations;
--- 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;
--- a/api/trie.h
+++ b/api/trie.h
@ -76,7 +76,7 @@ public:
                {
                    node = new trienode < typename Col::value_type >;
                }
-                catch( std::bad_alloc e )
+                catch(const std::bad_alloc& )
                {
                    return( false );
                }
--- a/cli/ccommon.h
+++ b/cli/ccommon.h
@ -153,15 +153,15 @@ public:
        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 @@ public:
    CParameter*                       Parameter;
    TermDataType                      DataType;
    pictcli_constraints::RelationType RelationType;
-    void*                             Data;
    std::wstring                      RawText;
+    void*                             Data;
 };

 //
--- 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 );
    }
--- 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
--- a/cli/model.h
+++ b/cli/model.h
@ -23,8 +23,8 @@ public:
        unsigned int weight,
        bool         positive
        ) : _names           ( names ),
-            _weight          ( weight ),
            _positive        ( positive ),
+            _weight          ( weight ),
            _currentNameIndex( 0 ) {}

    wstrings& GetAllNames()       { return( _names ); }
@ -60,8 +60,8 @@ public:

    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 @@ public:
        CaseSensitive(false),
        Verbose(false),
        Statistics(false),
-        RowSeedsFile(L""),
        GenerationMode(GenerationMode::Regular),
        MaxApproxTries(1000),
+        RowSeedsFile(L""),
        ConstraintPredicates(L""),
        m_hasNegativeValues(false),
        m_encoding(EncodingType::ANSI),
--- 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();
    }