Merge pull request #26548 from ZhilkinSerg/sa-2018-11-05

Code optimizations reported by static code analysis (2018-11-05)

src/activity_handlers.cpp

@@ -278,7 +278,6 @@ void set_up_butchery( player_activity &act, player &u, butcher_type action )
                 u.add_msg_if_player( m_info, _( "None of your tools are sharp and precise enough to do that." ) );
                 act.set_to_null();
                 return;
-                break;
             case 1:
                 u.add_msg_if_player( m_info, _( "You could use a better tool, but this will do." ) );
                 break;
@@ -1209,7 +1208,6 @@ void activity_handlers::butcher_finish( player_activity *act, player *p )
             }
             act->set_to_null();
             return;
-            break;
         case DISSECT:
             p->add_msg_if_player( m_good, _( "You finish dissecting the %s." ), corpse_item.tname().c_str() );
             g->m.i_rem( p->pos(), act->index );

src/bonuses.cpp

@@ -127,11 +127,10 @@ void bonus_container::load( JsonArray &jarr, bool mult )
     while( jarr.has_more() ) {
         JsonArray qualifiers = jarr.next_array();
 
-        affected_stat as;
         damage_type dt = DT_NULL;
 
         const std::string affected_stat_string = qualifiers.next_string();
-        as = affected_stat_from_string( affected_stat_string );
+        affected_stat as = affected_stat_from_string( affected_stat_string );
         if( as == AFFECTED_NULL ) {
             jarr.throw_error( "Invalid affected stat" );
         }

src/catacharset.cpp

@@ -213,13 +213,12 @@ int cursorx_to_position( const char *line, int cursorx, int *prevpos, int maxlen
 
 std::string utf8_truncate( std::string s, size_t length )
 {
-    int last_pos;
 
     if( length == 0 || s.empty() ) {
         return s;
     }
 
-    last_pos = cursorx_to_position( s.c_str(), length, NULL, -1 );
+    int last_pos = cursorx_to_position( s.c_str(), length, NULL, -1 );
 
     return s.substr( 0, last_pos );
 }

src/clzones.cpp

@@ -232,7 +232,7 @@ void plot_options::deserialize( JsonObject &jo_zone )
     seed = jo_zone.get_string( "seed", "" );
 };
 
-cata::optional<std::string> zone_manager::query_name( std::string default_name ) const
+cata::optional<std::string> zone_manager::query_name( const std::string &default_name ) const
 {
     string_input_popup popup;
     popup

src/clzones.h

@@ -172,7 +172,7 @@ class zone_manager
         std::vector<zone_data> get_zones( const zone_type_id &type, const tripoint &where ) const;
         const zone_data *get_top_zone( const tripoint &where ) const;
         const zone_data *get_bottom_zone( const tripoint &where ) const;
-        cata::optional<std::string> query_name( std::string default_name = "" ) const;
+        cata::optional<std::string> query_name( const std::string &default_name = "" ) const;
         cata::optional<zone_type_id> query_type() const;
         void swap( zone_data &a, zone_data &b );
 

src/construction.cpp

@@ -37,14 +37,10 @@
 
 static const skill_id skill_fabrication( "fabrication" );
 static const skill_id skill_electronics( "electronics" );
-static const skill_id skill_unarmed( "unarmed" );
-static const skill_id skill_throw( "throw" );
 
 static const trait_id trait_DEBUG_HS( "DEBUG_HS" );
-static const trait_id trait_NOPAIN( "NOPAIN" );
 static const trait_id trait_PAINRESIST_TROGLO( "PAINRESIST_TROGLO" );
 static const trait_id trait_STOCKY_TROGLO( "STOCKY_TROGLO" );
-static const trait_id trait_WEB_ROPE( "WEB_ROPE" );
 
 const trap_str_id tr_firewood_source( "tr_firewood_source" );
 

src/crafting_gui.cpp

@@ -57,7 +57,6 @@ std::string get_cat_name( const std::string &prefixed_name )
 
 void load_recipe_category( JsonObject &jsobj )
 {
-    JsonArray subcats;
     std::string category = jsobj.get_string( "id" );
 
     if( category.find( "CC_" ) != 0 ) {
@@ -69,7 +68,7 @@ void load_recipe_category( JsonObject &jsobj )
     std::string cat_name = get_cat_name( category );
 
     craft_subcat_list[category] = std::vector<std::string>();
-    subcats = jsobj.get_array( "recipe_subcategories" );
+    JsonArray subcats = jsobj.get_array( "recipe_subcategories" );
     while( subcats.has_more() ) {
         std::string subcat_id = subcats.next_string();
         if( subcat_id.find( "CSC_" + cat_name + "_" ) != 0 && subcat_id != "CSC_ALL" ) {

src/creature_tracker.cpp

@@ -129,8 +129,6 @@ bool Creature_tracker::update_pos( const monster &critter, const tripoint &new_p
         rebuild_cache();
         return false;
     }
-
-    return false;
 }
 
 void Creature_tracker::remove_from_location_map( const monster &critter )

src/dependency_tree.h

@@ -77,7 +77,6 @@ class dependency_tree
         dependency_node *get_node( mod_id key );
 
         std::map<mod_id, dependency_node> master_node_map;
-    protected:
     private:
         // Don't need to be called directly. Only reason to call these are during initialization phase.
         void build_node_map( std::map<mod_id, std::vector<mod_id > > key_dependency_map );

src/dialogue.h

@@ -153,7 +153,7 @@ struct talk_response {
                 /**
                  * Sets an effect to a function object and consequence to explicitly given one.
                  */
-                void set_effect_consequence( effect_fun_t eff, dialogue_consequence con );
+                void set_effect_consequence( const effect_fun_t &eff, dialogue_consequence con );
                 void set_effect_consequence( std::function<void( npc &p )> ptr, dialogue_consequence con );
 
 
@@ -349,8 +349,6 @@ class json_talk_response
  */
 class json_talk_topic
 {
-    public:
-
     private:
         bool replace_built_in_responses = false;
         std::vector<json_talk_response> responses;

src/dialogue_win.cpp

@@ -109,8 +109,7 @@ bool dialogue_window::print_responses( int const yoffset, const std::vector<talk
     size_t const fold_width = xoffset - 2 - 2 - 2;
 
     int curline = min_line - ( int ) yoffset;
-    size_t i;
-    for( i = 0; i < responses.size() && curline <= max_line; i++ ) {
+    for( size_t i = 0; i < responses.size() && curline <= max_line; i++ ) {
         const std::vector<std::string> folded = foldstring( responses[i].second, fold_width );
         const nc_color &color = responses[i].first;
         for( size_t j = 0; j < folded.size(); j++, curline++ ) {
@@ -138,7 +137,8 @@ void dialogue_window::refresh_response_display()
     can_scroll_up = false;
 }
 
-void dialogue_window::display_responses( int const hilight_lines, std::vector<talk_data> responses,
+void dialogue_window::display_responses( int const hilight_lines,
+        const std::vector<talk_data> &responses,
         const long &ch )
 {
     if( text_only ) {

src/dialogue_win.h

@@ -18,7 +18,8 @@ class dialogue_window
         bool text_only = false;
 
         void clear_window_texts();
-        void display_responses( int hilight_lines, std::vector<talk_data> responses, const long &ch );
+        void display_responses( int hilight_lines, const std::vector<talk_data> &responses,
+                                const long &ch );
         void refresh_response_display();
         /**
          * Folds and adds the folded text to @ref history. Returns the number of added lines.

src/faction_camp.cpp

@@ -2302,7 +2302,7 @@ std::map<std::string, std::string> talk_function::camp_recipe_deck( const std::s
     return cooking_recipes;
 }
 
-int talk_function::camp_recipe_batch_max( const recipe making, const inventory &total_inv )
+int talk_function::camp_recipe_batch_max( const recipe &making, const inventory &total_inv )
 {
     int max_batch = 0;
     int max_checks = 9;

src/faction_camp.h

@@ -222,7 +222,7 @@ std::string name_mission_tabs( npc &p, const std::string &id, const std::string
 /// Creats a map of all the recipes that are available to a building at om_cur, "ALL" for all possible
 std::map<std::string, std::string> camp_recipe_deck( const std::string &om_cur );
 /// Determines what the absolute max (out of 9999) that can be crafted using inventory and food supplies
-int camp_recipe_batch_max( const recipe making, const inventory &total_inv );
+int camp_recipe_batch_max( const recipe &making, const inventory &total_inv );
 
 /*
  * check if a companion survives a random encounter

src/game.cpp

@@ -4447,11 +4447,10 @@ int game::mon_info( const catacurses::window &w )
 
     // Print the symbols of all monsters in all directions.
     for( int i = 0; i < 8; i++ ) {
-        int symroom;
         point pr( xcoords[i] + widths[i] + 1, ycoords[i] + startrow );
 
         // The list of symbols needs a space on each end.
-        symroom = ( width / 3 ) - widths[i] - 2;
+        int symroom = ( width / 3 ) - widths[i] - 2;
         const int typeshere_npc = unique_types[i].size();
         const int typeshere_mon = unique_mons[i].size();
         const int typeshere = typeshere_mon + typeshere_npc;
@@ -5154,7 +5153,6 @@ void game::emp_blast( const tripoint &p )
     // TODO: Implement z part
     int x = p.x;
     int y = p.y;
-    int rn;
     const bool sight = g->u.sees( p );
     if( m.has_flag( "CONSOLE", x, y ) ) {
         if( sight ) {
@@ -5165,7 +5163,7 @@ void game::emp_blast( const tripoint &p )
     }
     // TODO: More terrain effects.
     if( m.ter( x, y ) == t_card_science || m.ter( x, y ) == t_card_military ) {
-        rn = rng( 1, 100 );
+        int rn = rng( 1, 100 );
         if( rn > 92 || rn < 40 ) {
             if( sight ) {
                 add_msg( _( "The card reader is rendered non-functional." ) );
@@ -5632,7 +5630,7 @@ void game::exam_vehicle( vehicle &veh, int cx, int cy )
     }
 }
 
-bool game::forced_door_closing( const tripoint &p, const ter_id door_type, int bash_dmg )
+bool game::forced_door_closing( const tripoint &p, const ter_id &door_type, int bash_dmg )
 {
     // TODO: Z
     const int &x = p.x;
@@ -6545,9 +6543,8 @@ void game::print_fields_info( const tripoint &lp, const catacurses::window &w_lo
         if( fld.first == fd_fire && ( m.has_flag( TFLAG_FIRE_CONTAINER, lp ) ||
                                       m.ter( lp ) == t_pit_shallow || m.ter( lp ) == t_pit ) ) {
             const int max_width = getmaxx( w_look ) - column - 2;
-            int lines;
-            lines = fold_and_print( w_look, ++line, column, max_width, cur.color(),
-                                    get_fire_fuel_string( lp ) ) - 1;
+            int lines = fold_and_print( w_look, ++line, column, max_width, cur.color(),
+                                        get_fire_fuel_string( lp ) ) - 1;
             line += lines;
         } else {
             mvwprintz( w_look, ++line, column, cur.color(), cur.name() );
@@ -8817,8 +8814,8 @@ void game::plthrow( int pos )
     m.draw( w_terrain, u.pos() );
 
     // target_ui() sets x and y, or returns empty vector if we canceled (by pressing Esc)
-    std::vector<tripoint> trajectory;
-    trajectory = target_handler().target_ui( u, TARGET_MODE_THROW, &thrown, range );
+    std::vector<tripoint> trajectory = target_handler().target_ui( u, TARGET_MODE_THROW, &thrown,
+                                       range );
     if( trajectory.empty() ) {
         return;
     }
@@ -10894,9 +10891,8 @@ void game::place_player( const tripoint &dest_loc )
                 }
                 int and_the_rest = 0;
                 for( size_t i = 0; i < names.size(); ++i ) {
-                    std::string fmt;
                     //~ number of items: "<number> <item>"
-                    fmt = ngettext( "%1$d %2$s", "%1$d %2$s", counts[i] );
+                    std::string fmt = ngettext( "%1$d %2$s", "%1$d %2$s", counts[i] );
                     names[i] = string_format( fmt, counts[i], names[i].c_str() );
                     // Skip the first two.
                     if( i > 1 ) {
@@ -12589,8 +12585,7 @@ void game::process_artifact( item &it, player &p )
 {
     const bool worn = p.is_worn( it );
     const bool wielded = ( &it == &p.weapon );
-    std::vector<art_effect_passive> effects;
-    effects = it.type->artifact->effects_carried;
+    std::vector<art_effect_passive> effects = it.type->artifact->effects_carried;
     if( worn ) {
         auto &ew = it.type->artifact->effects_worn;
         effects.insert( effects.end(), ew.begin(), ew.end() );

src/game.h

@@ -850,7 +850,7 @@ class game
         // will do so, if bash_dmg is greater than 0, items won't stop the door
         // from closing at all.
         // If the door gets closed the items on the door tile get moved away or destroyed.
-        bool forced_door_closing( const tripoint &p, const ter_id door_type, int bash_dmg );
+        bool forced_door_closing( const tripoint &p, const ter_id &door_type, int bash_dmg );
 
         //pixel minimap management
         int pixel_minimap_option;

src/gamemode.h

@@ -138,14 +138,12 @@ struct defense_game : public special_game {
 
     private:
         void init_to_style( defense_style new_style );
-        void load_style( std::string style_name );
 
         void setup();
         void refresh_setup( const catacurses::window &w, int selection );
         void init_mtypes();
         void init_constructions();
         void init_map();
-        std::vector<itype_id> carvan_items( caravan_category cat );
 
         void spawn_wave();
         void caravan();

src/iexamine.cpp

@@ -68,7 +68,6 @@ const efftype_id effect_sleep( "sleep" );
 static const trait_id trait_AMORPHOUS( "AMORPHOUS" );
 static const trait_id trait_ARACHNID_ARMS_OK( "ARACHNID_ARMS_OK" );
 static const trait_id trait_BADKNEES( "BADKNEES" );
-static const trait_id trait_BEAK_HUM( "BEAK_HUM" );
 static const trait_id trait_ILLITERATE( "ILLITERATE" );
 static const trait_id trait_INSECT_ARMS_OK( "INSECT_ARMS_OK" );
 static const trait_id trait_M_DEFENDER( "M_DEFENDER" );
@@ -77,7 +76,6 @@ static const trait_id trait_M_FERTILE( "M_FERTILE" );
 static const trait_id trait_M_SPORES( "M_SPORES" );
 static const trait_id trait_NOPAIN( "NOPAIN" );
 static const trait_id trait_PARKOUR( "PARKOUR" );
-static const trait_id trait_PROBOSCIS( "PROBOSCIS" );
 static const trait_id trait_THRESH_MARLOSS( "THRESH_MARLOSS" );
 static const trait_id trait_THRESH_MYCUS( "THRESH_MYCUS" );
 static const trait_id trait_BURROW( "BURROW" );
@@ -4127,10 +4125,8 @@ void iexamine::smoker_options( player &p, const tripoint &examp )
         case 1: //activate
             if ( active ) {
                 add_msg( _("It is already lit and smoking.") );
-                break;
             } else {
                 smoker_activate( p, examp );
-                break;
             }
             break;
         case 2: // load food

src/input.cpp

@@ -1343,7 +1343,7 @@ std::vector<std::string> input_context::filter_strings_by_phrase(
     return filtered_strings;
 }
 
-void input_context::set_edittext( std::string s )
+void input_context::set_edittext( const std::string &s )
 {
     edittext = s;
 }

src/input.h

@@ -264,8 +264,6 @@ class input_manager
          */
         void wait_for_any_key();
 
-        bool translate_to_window_position();
-
         /**
          * Sets global input polling timeout as appropriate for the current interface system.
          * Use `input_context::(re)set_timeout()` when possible so timeout will be properly
@@ -646,7 +644,7 @@ class input_context
         /**
         * Get/Set edittext to display IME unspecified string.
         */
-        void set_edittext( std::string s );
+        void set_edittext( const std::string &s );
         std::string get_edittext();
 
         void set_iso( bool mode = true );

src/inventory_ui.cpp

@@ -1263,7 +1263,7 @@ inventory_selector::stat display_stat( const std::string &caption, int cur_value
 
 inventory_selector::stats inventory_selector::get_weight_and_volume_stats(
     units::mass weight_carried, units::mass weight_capacity,
-    units::volume volume_carried, units::volume volume_capacity )
+    const units::volume &volume_carried, const units::volume &volume_capacity )
 {
     return {
         {

src/inventory_ui.h

@@ -492,7 +492,7 @@ class inventory_selector
 
         static stats get_weight_and_volume_stats(
             units::mass weight_carried, units::mass weight_capacity,
-            units::volume volume_carried, units::volume volume_capacity );
+            const units::volume &volume_carried, const units::volume &volume_capacity );
 
         /** Get stats to display in top right.
          *
@@ -551,7 +551,6 @@ class inventory_selector
             }
         }
         void toggle_navigation_mode();
-        void reassign_custom_invlets();
 
         /** Entry has been added */
         virtual void on_entry_add( const inventory_entry & ) {}

src/item.cpp

@@ -626,7 +626,7 @@ bool item::merge_charges( const item &rhs )
     return true;
 }
 
-void item::put_in( item payload )
+void item::put_in( const item &payload )
 {
     contents.push_back( payload );
 }

src/item.h

@@ -624,7 +624,7 @@ class item : public visitable<item>
         /**
          * Puts the given item into this one, no checks are performed.
          */
-        void put_in( item payload );
+        void put_in( const item &payload );
 
         /** Stores a newly constructed item at the end of this item's contents */
         template<typename ... Args>

src/item_factory.h

@@ -331,7 +331,6 @@ class Item_factory
         void add_entry( Item_group &sg, JsonObject &obj );
 
         void load_basic_info( JsonObject &jo, itype &def, const std::string &src );
-        void tags_from_json( JsonObject &jo, std::string member, std::set<std::string> &tags );
         void set_qualities_from_json( JsonObject &jo, const std::string &member, itype &def );
         void set_properties_from_json( JsonObject &jo, const std::string &member, itype &def );