Merge pull request #27539 from ZhilkinSerg/sa-2019-01-11

Code optimizations reported by static code analysis (2019-01-11)

src/activity_handlers.cpp

@@ -2978,7 +2978,7 @@ void activity_handlers::fertilize_plot_do_turn( player_activity *act, player *p
         }
     };
 
-    auto have_fertilizer = [&]( void ) {
+    auto have_fertilizer = [&]() {
         return !fertilizer.empty() && p->has_charges( fertilizer, 1 );
     };
 

src/bonuses.cpp

@@ -90,9 +90,7 @@ std::string string_from_scaling_stat( const scaling_stat &s )
     return iter != scaling_stat_map_translation.end() ? _( iter->second.c_str() ) : "";
 }
 
-bonus_container::bonus_container()
-{
-}
+bonus_container::bonus_container() = default;
 
 void effect_scaling::load( JsonArray &jarr )
 {

src/coordinates.h

@@ -31,8 +31,7 @@ struct real_coords {
     point om_pos;      // overmap tile: 2x2 submaps.
     point om_sub;      // submap (0-359) in overmap / abs_sub constrained to % 360. equivalent to g->levx
 
-    real_coords() {
-    }
+    real_coords() = default;
 
     real_coords( point ap ) {
         fromabs( ap.x, ap.y );

src/creature_tracker.cpp

@@ -11,9 +11,7 @@
 
 #define dbg(x) DebugLog((DebugLevel)(x),D_GAME) << __FILE__ << ":" << __LINE__ << ": "
 
-Creature_tracker::Creature_tracker()
-{
-}
+Creature_tracker::Creature_tracker() = default;
 
 Creature_tracker::~Creature_tracker() = default;
 

src/damage.cpp

@@ -20,7 +20,7 @@ bool damage_unit::operator==( const damage_unit &other ) const
            damage_multiplier == other.damage_multiplier;
 }
 
-damage_instance::damage_instance() { }
+damage_instance::damage_instance() = default;
 damage_instance damage_instance::physical( float bash, float cut, float stab, float arpen )
 {
     damage_instance d;

src/debug.cpp

@@ -292,9 +292,7 @@ static std::ostream nullStream( &nullBuf );
 
 static DebugFile debugFile;
 
-DebugFile::DebugFile()
-{
-}
+DebugFile::DebugFile() = default;
 
 DebugFile::~DebugFile()
 {

src/explosion.cpp

@@ -321,7 +321,9 @@ void game::explosion( const tripoint &p, const explosion_data &ex )
             int qty = shr.casing_mass * std::min( 1.0, shr.recovery / 100.0 ) /
                       to_gram( fragment_drop->weight );
             // Truncate to a random selection
-            std::random_shuffle( tiles.begin(), tiles.end() );
+            static auto eng = std::default_random_engine(
+                                  std::chrono::system_clock::now().time_since_epoch().count() );
+            std::shuffle( tiles.begin(), tiles.end(), eng );
             tiles.resize( std::min( int( tiles.size() ), qty ) );
 
             for( const auto &e : tiles ) {

src/fire.h

@@ -16,7 +16,7 @@
  * this turn.
  */
 struct fire_data {
-    fire_data() {}
+    fire_data() = default;
     fire_data( const fire_data & ) = default;
     fire_data( int intensity, bool is_contained = false ) : fire_intensity( intensity ),
         contained( is_contained )

src/item.cpp

@@ -1626,7 +1626,7 @@ std::string item::info( std::vector<iteminfo> &info, const iteminfo_query *parts
             temp1.str( "" );
             temp1 << _( "<bold>Incompatible with mod location: </bold> " );
             int iternum = 0;
-            for( auto black : mod.blacklist_mod ) {
+            for( const auto &black : mod.blacklist_mod ) {
                 if( iternum != 0 ) {
                     temp1 << ", ";
                 }
@@ -5357,7 +5357,7 @@ ret_val<bool> item::is_gunmod_compatible( const item &mod ) const
         return ret_val<bool>::make_failure( _( "must be unloaded before installing this mod" ) );
     }
 
-    for( auto slot : mod.type->gunmod->blacklist_mod ) {
+    for( const auto &slot : mod.type->gunmod->blacklist_mod ) {
         if( get_mod_locations().count( slot ) ) {
             return ret_val<bool>::make_failure( _( "cannot be installed on a weapon with \"%s\"" ),
                                                 slot.name().c_str() );

src/item_group.cpp

@@ -207,7 +207,7 @@ void Item_modifier::modify( item &new_item ) const
     }
 
     if( ch > 0 && ( new_item.is_gun() || new_item.is_magazine() ) ) {
-        if( ammo.get() == nullptr ) {
+        if( ammo == nullptr ) {
             // In case there is no explicit ammo item defined, use the default ammo
             if( new_item.ammo_type() ) {
                 new_item.ammo_set( new_item.ammo_type()->default_ammotype(), ch );
@@ -235,7 +235,7 @@ void Item_modifier::modify( item &new_item ) const
         }
 
         if( spawn_ammo ) {
-            if( ammo.get() ) {
+            if( ammo ) {
                 const item am = ammo->create_single( new_item.birthday() );
                 new_item.ammo_set( am.typeId() );
             } else {
@@ -244,7 +244,7 @@ void Item_modifier::modify( item &new_item ) const
         }
     }
 
-    if( container.get() != nullptr ) {
+    if( container != nullptr ) {
         item cont = container->create_single( new_item.birthday() );
         if( !cont.is_null() ) {
             if( new_item.made_of( LIQUID ) ) {
@@ -260,7 +260,7 @@ void Item_modifier::modify( item &new_item ) const
         }
     }
 
-    if( contents.get() != nullptr ) {
+    if( contents != nullptr ) {
         Item_spawn_data::ItemList contentitems = contents->create( new_item.birthday() );
         new_item.contents.insert( new_item.contents.end(), contentitems.begin(), contentitems.end() );
     }
@@ -272,10 +272,10 @@ void Item_modifier::modify( item &new_item ) const
 
 void Item_modifier::check_consistency() const
 {
-    if( ammo.get() != nullptr ) {
+    if( ammo != nullptr ) {
         ammo->check_consistency();
     }
-    if( container.get() != nullptr ) {
+    if( container != nullptr ) {
         container->check_consistency();
     }
     if( with_ammo < 0 || with_ammo > 100 ) {
@@ -288,12 +288,12 @@ void Item_modifier::check_consistency() const
 
 bool Item_modifier::remove_item( const Item_tag &itemid )
 {
-    if( ammo.get() != nullptr ) {
+    if( ammo != nullptr ) {
         if( ammo->remove_item( itemid ) ) {
             ammo.reset();
         }
     }
-    if( container.get() != nullptr ) {
+    if( container != nullptr ) {
         if( container->remove_item( itemid ) ) {
             container.reset();
             return true;

src/itype.h

@@ -671,7 +671,7 @@ struct islot_seed {
      */
     std::vector<std::string> byproducts;
 
-    islot_seed() { }
+    islot_seed() = default;
 };
 
 struct islot_artifact {

src/iuse.h

@@ -300,7 +300,7 @@ struct use_function {
         }
 
         explicit operator bool() const {
-            return actor.get() != nullptr;
+            return actor != nullptr;
         }
 
         /** @return See @ref iuse_actor::type */

src/iuse_software_snake.cpp

@@ -12,9 +12,7 @@
 #include "string_formatter.h"
 #include "translations.h"
 
-snake_game::snake_game()
-{
-}
+snake_game::snake_game() = default;
 
 void snake_game::print_score( const catacurses::window &w_snake, int iScore )
 {

src/iuse_software_sokoban.cpp

@@ -12,9 +12,7 @@
 #include "string_formatter.h"
 #include "translations.h"
 
-sokoban_game::sokoban_game()
-{
-}
+sokoban_game::sokoban_game() = default;
 
 void sokoban_game::print_score( const catacurses::window &w_sokoban, int iScore, int iMoves )
 {

src/json.cpp

@@ -927,7 +927,7 @@ std::string JsonIn::get_member_name()
 
 std::string JsonIn::get_string()
 {
-    std::string s = "";
+    std::string s;
     char ch;
     bool backslash = false;
     char unihex[5] = "0000";

src/json.h

@@ -981,9 +981,9 @@ std::set<T> JsonObject::get_tags( const std::string &name )
 class JsonSerializer
 {
     public:
-        virtual ~JsonSerializer() {}
+        virtual ~JsonSerializer() = default;
         virtual void serialize( JsonOut &jsout ) const = 0;
-        JsonSerializer() { }
+        JsonSerializer() = default;
         JsonSerializer( JsonSerializer && ) = default;
         JsonSerializer( const JsonSerializer & ) = default;
         JsonSerializer &operator=( JsonSerializer && ) = default;

src/lightmap.cpp

@@ -1130,8 +1130,7 @@ void map::build_seen_cache( const tripoint &origin, const int target_z )
         }
     }
 
-    for( size_t i = 0; i < mirrors.size(); i++ ) {
-        const int &mirror = mirrors[i];
+    for( int mirror : mirrors ) {
         bool is_camera = veh->part_info( mirror ).has_flag( "CAMERA" );
         if( is_camera && cam_control < 0 ) {
             continue; // Player not at camera control, so cameras don't work

src/mapbuffer.cpp

@@ -21,9 +21,7 @@
 
 mapbuffer MAPBUFFER;
 
-mapbuffer::mapbuffer()
-{
-}
+mapbuffer::mapbuffer() = default;
 
 mapbuffer::~mapbuffer()
 {

src/mapgen.cpp

@@ -2,7 +2,9 @@
 
 #include <algorithm>
 #include <cassert>
+#include <chrono>
 #include <list>
+#include <random>
 #include <sstream>
 
 #include "ammo.h"
@@ -238,23 +240,23 @@ std::map<std::string, std::map<int, int> > oter_mapgen_weights;
 void calculate_mapgen_weights()   // @todo: rename as it runs jsonfunction setup too
 {
     oter_mapgen_weights.clear();
-    for( auto oit = oter_mapgen.begin(); oit != oter_mapgen.end(); ++oit ) {
+    for( auto &omw : oter_mapgen ) {
         int funcnum = 0;
         int wtotal = 0;
-        oter_mapgen_weights[ oit->first ] = std::map<int, int>();
-        for( auto fit = oit->second.begin(); fit != oit->second.end(); ++fit ) {
+        oter_mapgen_weights[ omw.first ] = std::map<int, int>();
+        for( auto fit = omw.second.begin(); fit != omw.second.end(); ++fit ) {
             //
             int weight = ( *fit )->weight;
             if( weight < 1 ) {
-                dbg( D_INFO ) << "wcalc " << oit->first << "(" << funcnum << "): (rej(1), " << weight << ") = " <<
+                dbg( D_INFO ) << "wcalc " << omw.first << "(" << funcnum << "): (rej(1), " << weight << ") = " <<
                               wtotal;
                 ++funcnum;
                 continue; // rejected!
             }
             ( *fit )->setup();
             wtotal += weight;
-            oter_mapgen_weights[ oit->first ][ wtotal ] = funcnum;
-            dbg( D_INFO ) << "wcalc " << oit->first << "(" << funcnum << "): +" << weight << " = " << wtotal;
+            oter_mapgen_weights[ omw.first ][ wtotal ] = funcnum;
+            dbg( D_INFO ) << "wcalc " << omw.first << "(" << funcnum << "): +" << weight << " = " << wtotal;
             ++funcnum;
         }
     }
@@ -5894,7 +5896,9 @@ FFFFFFFFFFFFFFFFFFFFFFf \n\
             for( int a = 0; a < 21; a++ ) {
                 vset.push_back( a );
             }
-            std::random_shuffle( vset.begin(), vset.end() );
+            static auto eng = std::default_random_engine(
+                                  std::chrono::system_clock::now().time_since_epoch().count() );
+            std::shuffle( vset.begin(), vset.end(), eng );
             for( int a = 0; a < vnum; a++ ) {
                 if( vset[a] < 12 ) {
                     if( one_in( 2 ) ) {

src/mapgen.h

@@ -146,7 +146,7 @@ struct jmapgen_setmap {
 class jmapgen_piece
 {
     protected:
-        jmapgen_piece() { }
+        jmapgen_piece() = default;
     public:
         /** Place something on the map from mapgendata dat, at (x,y). mon_density */
         virtual void apply( const mapgendata &dat, const jmapgen_int &x, const jmapgen_int &y,

src/messages.cpp

@@ -389,9 +389,9 @@ void Messages::dialog::init()
         const size_t msg_ind = log_from_top ? ind : msg_count - 1 - ind;
         const game_message &msg = player_messages.history( msg_ind );
         const auto &folded = foldstring( msg.get_with_count(), msg_width );
-        for( auto it = folded.begin(); it != folded.end(); ++it ) {
+        for( const auto &it : folded ) {
             folded_filtered.emplace_back( folded_all.size() );
-            folded_all.emplace_back( msg_ind, *it );
+            folded_all.emplace_back( msg_ind, it );
         }
     }
 

src/mission_companion.cpp

@@ -1761,7 +1761,7 @@ std::vector<comp_rank> talk_function::companion_rank( const std::vector<std::sha
     int max_combat = 0;
     int max_survival = 0;
     int max_industry = 0;
-    for( auto e : available ) {
+    for( const auto &e : available ) {
         comp_rank r;
         r.combat = companion_combat_rank( *e );
         r.survival = companion_survival_rank( *e );

src/mission_ui.cpp

@@ -103,7 +103,7 @@ void game::list_missions()
         if( selection < umissions.size() ) {
             const auto miss = umissions[selection];
             const nc_color col = u.get_active_mission() == miss ? c_light_green : c_white;
-            std::string for_npc = "";
+            std::string for_npc;
             if( miss->get_npc_id() >= 0 ) {
                 npc *guy = g->find_npc( miss->get_npc_id() );
                 if( guy ) {

src/monattack.cpp

@@ -240,18 +240,18 @@ bool mattack::eat_food( monster *z )
             continue;
         }
         auto items = g->m.i_at( p );
-        for( auto i = items.begin(); i != items.end(); i++ ) {
+        for( auto &item : items ) {
             //Fun limit prevents scavengers from eating feces
-            if( !i->is_food() || i->type->comestible->fun < -20 ) {
+            if( !item.is_food() || item.type->comestible->fun < -20 ) {
                 continue;
             }
             //Don't eat own eggs
-            if( z->type->baby_egg != i->type->get_id() ) {
+            if( z->type->baby_egg != item.type->get_id() ) {
                 long consumed = 1;
-                if( i->count_by_charges() ) {
-                    g->m.use_charges( p, 0, i->type->get_id(), consumed );
+                if( item.count_by_charges() ) {
+                    g->m.use_charges( p, 0, item.type->get_id(), consumed );
                 } else {
-                    g->m.use_amount( p, 0, i->type->get_id(), consumed );
+                    g->m.use_amount( p, 0, item.type->get_id(), consumed );
                 }
                 return true;
             }

src/mongroup.cpp

@@ -450,11 +450,11 @@ const mtype_id &MonsterGroupManager::GetRandomMonsterFromGroup( const mongroup_i
 {
     const auto &group = group_name.obj();
     int spawn_chance = rng( 1, group.freq_total ); //Default 1000 unless specified
-    for( auto it = group.monsters.begin(); it != group.monsters.end(); ++it ) {
-        if( it->frequency >= spawn_chance ) {
-            return it->name;
+    for( const auto &monster_type : group.monsters ) {
+        if( monster_type.frequency >= spawn_chance ) {
+            return monster_type.name;
         } else {
-            spawn_chance -= it->frequency;
+            spawn_chance -= monster_type.frequency;
         }
     }
 

src/monster.cpp

@@ -1838,8 +1838,8 @@ void monster::process_turn()
             for( const tripoint &zap : g->m.points_in_radius( pos(), 1 ) ) {
                 const bool player_sees = g->u.sees( zap );
                 const auto items = g->m.i_at( zap );
-                for( auto fiyah = items.begin(); fiyah != items.end(); fiyah++ ) {
-                    if( fiyah->made_of( LIQUID ) && fiyah->flammable() ) { // start a fire!
+                for( const auto &item : items ) {
+                    if( item.made_of( LIQUID ) && item.flammable() ) { // start a fire!
                         g->m.add_field( zap, fd_fire, 2, 1_minutes );
                         sounds::sound( pos(), 30, sounds::sound_t::combat,  _( "fwoosh!" ) );
                         break;