Code optimizations reported by static code analysis (2020-01-14)

src/activity_handlers.cpp

@@ -352,8 +352,8 @@ static void butcher_cbm_item( const std::string &what, const tripoint &pos,
 }
 
 static void butcher_cbm_group( const std::string &group, const tripoint &pos,
-                               const time_point &age, const int roll, const std::vector<std::string> flags,
-                               const std::vector<fault_id> faults )
+                               const time_point &age, const int roll, const std::vector<std::string> &flags,
+                               const std::vector<fault_id> &faults )
 {
     if( roll < 0 ) {
         return;
@@ -2316,7 +2316,7 @@ static repeat_type repeat_menu( const std::string &title, repeat_type last_selec
     return REPEAT_CANCEL;
 }
 
-// This is a part of a hack to provide pseudo items for long repair activity
+// HACK: This is a part of a hack to provide pseudo items for long repair activity
 // Note: similar hack could be used to implement all sorts of vehicle pseudo-items
 //  and possibly CBM pseudo-items too.
 struct weldrig_hack {

src/activity_item_handling.cpp

@@ -905,7 +905,7 @@ static int move_cost( const item &it, const tripoint &src, const tripoint &dest
     return move_cost_inv( it, src, dest );
 }
 
-static void vehicle_activity( player &p, const tripoint src_loc, int vpindex, char type )
+static void vehicle_activity( player &p, const tripoint &src_loc, int vpindex, char type )
 {
     vehicle *veh = veh_pointer_or_null( g->m.veh_at( src_loc ) );
     if( !veh ) {
@@ -1034,7 +1034,7 @@ static activity_reason_info find_base_construction(
     player &p,
     const inventory &inv,
     const tripoint &loc,
-    const cata::optional<size_t> part_con_idx,
+    const cata::optional<size_t> &part_con_idx,
     const size_t idx,
     std::set<size_t> &used )
 {
@@ -1854,7 +1854,7 @@ static std::vector<std::tuple<tripoint, itype_id, int>> requirements_map( player
     return final_map;
 }
 
-static void construction_activity( player &p, const zone_data *zone, const tripoint src_loc,
+static void construction_activity( player &p, const zone_data *zone, const tripoint &src_loc,
                                    const activity_reason_info &act_info, const std::vector<construction> &list_constructions,
                                    activity_id activity_to_restore )
 {
@@ -1962,7 +1962,7 @@ static void fetch_activity( player &p, const tripoint &src_loc,
     std::string picked_up;
     const units::volume volume_allowed = p.volume_capacity() - p.volume_carried();
     const units::mass weight_allowed = p.weight_capacity() - p.weight_carried();
-    // TODO : vehicle_stack and map_stack into one loop.
+    // TODO: vehicle_stack and map_stack into one loop.
     if( src_veh ) {
         for( auto &veh_elem : src_veh->get_items( src_part ) ) {
             for( auto elem : mental_map_2 ) {

src/advanced_inv.cpp

@@ -261,7 +261,7 @@ void advanced_inventory::init()
     panes[right].window = right_window;
 }
 
-void advanced_inventory::print_items( advanced_inventory_pane &pane, bool active )
+void advanced_inventory::print_items( const advanced_inventory_pane &pane, bool active )
 {
     const auto &items = pane.items;
     const catacurses::window &window = pane.window;

src/advanced_inv.h

@@ -129,7 +129,7 @@ class advanced_inventory
 
         static std::string get_sortname( advanced_inv_sortby sortby );
         bool move_all_items( bool nested_call = false );
-        void print_items( advanced_inventory_pane &pane, bool active );
+        void print_items( const advanced_inventory_pane &pane, bool active );
         void recalc_pane( side p );
         void redraw_pane( side p );
         // Returns the x coordinate where the header started. The header is

src/advanced_inv_pane.h

@@ -41,7 +41,7 @@ class advanced_inventory_pane
         bool prev_viewing_cargo = false;
     public:
         // set the pane's area via its square, and whether it is viewing a vehicle's cargo
-        void set_area( advanced_inv_area &square, bool in_vehicle_cargo = false ) {
+        void set_area( const advanced_inv_area &square, bool in_vehicle_cargo = false ) {
             prev_area = area;
             prev_viewing_cargo = viewing_cargo;
             area = square.id;

src/auto_pickup.cpp

@@ -576,7 +576,7 @@ void rule_list::create_rule( cache &map_items, const std::string &to_match )
 
 void player_settings::create_rule( const item *it )
 {
-    // @TODO: change it to be a reference
+    // TODO: change it to be a reference
     global_rules.create_rule( map_items, *it );
     character_rules.create_rule( map_items, *it );
 }

src/ballistics.cpp

@@ -73,7 +73,7 @@ static void drop_or_embed_projectile( const dealt_projectile_attack &attack )
             g->m.add_item_or_charges( pt, i );
         }
 
-        //TODO: Sound
+        // TODO: Sound
         return;
     }
 

src/basecamp.cpp

@@ -263,7 +263,7 @@ bool basecamp::has_provides( const std::string &req, const expansion_data &e_dat
     return false;
 }
 
-bool basecamp::has_provides( const std::string &req, const cata::optional<point> dir,
+bool basecamp::has_provides( const std::string &req, const cata::optional<point> &dir,
                              int level ) const
 {
     if( !dir ) {

src/basecamp.h

@@ -156,7 +156,7 @@ class basecamp
 
         // upgrade levels
         bool has_provides( const std::string &req, const expansion_data &e_data, int level = 0 ) const;
-        bool has_provides( const std::string &req, cata::optional<point> dir = cata::nullopt,
+        bool has_provides( const std::string &req, const cata::optional<point> &dir = cata::nullopt,
                            int level = 0 ) const;
         void update_resources( const std::string &bldg );
         void update_provides( const std::string &bldg, expansion_data &e_data );

src/bionics.cpp

@@ -2502,17 +2502,17 @@ void finalize_bionics()
     }
 }
 
-void bionic::set_flag( const std::string flag )
+void bionic::set_flag( const std::string &flag )
 {
     bionic_tags.insert( flag );
 }
 
-void bionic::remove_flag( const std::string flag )
+void bionic::remove_flag( const std::string &flag )
 {
     bionic_tags.erase( flag );
 }
 
-bool bionic::has_flag( const std::string flag ) const
+bool bionic::has_flag( const std::string &flag ) const
 {
     return bionic_tags.find( flag ) != bionic_tags.end();
 }

src/bionics.h

@@ -160,9 +160,9 @@ struct bionic {
             return *id;
         }
 
-        void set_flag( std::string flag );
-        void remove_flag( std::string flag );
-        bool has_flag( std::string flag ) const;
+        void set_flag( const std::string &flag );
+        void remove_flag( const std::string &flag );
+        bool has_flag( const std::string &flag ) const;
 
         int get_quality( const quality_id &quality ) const;
 

src/calendar.cpp

@@ -118,7 +118,7 @@ time_point night_time( const time_point &p )
 
 time_point daylight_time( const time_point &p )
 {
-    // @TODO: Actual dailight should start 18 degrees before sunrise
+    // TODO: Actual dailight should start 18 degrees before sunrise
     return sunrise( p ) + 15_minutes;
 }
 

src/cata_tiles.h

@@ -79,7 +79,7 @@ class texture
         SDL_Rect srcrect = { 0, 0, 0, 0 };
 
     public:
-        texture( std::shared_ptr<SDL_Texture> ptr, const SDL_Rect rect ) : sdl_texture_ptr( ptr ),
+        texture( std::shared_ptr<SDL_Texture> ptr, const SDL_Rect &rect ) : sdl_texture_ptr( ptr ),
             srcrect( rect ) { }
         texture() = default;
 

src/character.cpp

@@ -1070,7 +1070,7 @@ bool Character::movement_mode_is( const character_movemode mode ) const
     return move_mode == mode;
 }
 
-void Character::add_effect( const efftype_id &eff_id, const time_duration dur, body_part bp,
+void Character::add_effect( const efftype_id &eff_id, const time_duration &dur, body_part bp,
                             bool permanent, int intensity, bool force, bool deferred )
 {
     Creature::add_effect( eff_id, dur, bp, permanent, intensity, force, deferred );
@@ -1459,7 +1459,7 @@ std::vector<itype_id> Character::get_fuel_available( const bionic_id &bio ) cons
     return stored_fuels;
 }
 
-int Character::get_fuel_capacity( const itype_id fuel ) const
+int Character::get_fuel_capacity( const itype_id &fuel ) const
 {
     int amount_stored = 0;
     if( !get_value( fuel ).empty() ) {
@@ -1478,7 +1478,7 @@ int Character::get_fuel_capacity( const itype_id fuel ) const
     return capacity - amount_stored;
 }
 
-int Character::get_total_fuel_capacity( const itype_id fuel ) const
+int Character::get_total_fuel_capacity( const itype_id &fuel ) const
 {
     int capacity = 0;
     for( const bionic_id &bid : get_bionics() ) {
@@ -4040,7 +4040,7 @@ void Character::update_bodytemp()
             // Morale bonus for comfiness - only if actually comfy (not too warm/cold)
             // Spread the morale bonus in time.
             if( comfortable_warmth > 0 &&
-                // @TODO: make this simpler and use time_duration/time_point
+                // TODO: make this simpler and use time_duration/time_point
                 to_turn<int>( calendar::turn ) % to_turns<int>( 1_minutes ) == to_turns<int>
                 ( 1_minutes * bp ) / to_turns<int>( 1_minutes * num_bp ) &&
                 get_effect_int( effect_cold, num_bp ) == 0 &&
@@ -4604,7 +4604,7 @@ nc_color Character::symbol_color() const
     return basic;
 }
 
-bool Character::is_immune_field( const field_type_id fid ) const
+bool Character::is_immune_field( const field_type_id &fid ) const
 {
     // Obviously this makes us invincible
     if( has_trait( debug_nodmg ) ) {
@@ -6449,8 +6449,7 @@ void Character::passive_absorb_hit( body_part bp, damage_unit &du ) const
     // >0 check because some mutations provide negative armor
     // Thin skin check goes before subdermal armor plates because SUBdermal
     if( du.amount > 0.0f ) {
-        // Horrible hack warning!
-        // Get rid of this as soon as CUT and STAB are split
+        // HACK: Get rid of this as soon as CUT and STAB are split
         if( du.type == DT_STAB ) {
             damage_unit du_copy = du;
             du_copy.type = DT_CUT;
@@ -7834,7 +7833,7 @@ bool Character::has_fire( const int quantity ) const
     } else if( has_bionic( bio_laser ) && get_power_level() > quantity * 5_kJ ) {
         return true;
     } else if( is_npc() ) {
-        // A hack to make NPCs use their Molotovs
+        // HACK: A hack to make NPCs use their Molotovs
         return true;
     }
     return false;

src/character.h

@@ -459,7 +459,7 @@ class Character : public Creature, public visitable<Character>
         virtual void set_movement_mode( character_movemode mode ) = 0;
 
         /** Performs any Character-specific modifications to the arguments before passing to Creature::add_effect(). */
-        void add_effect( const efftype_id &eff_id, time_duration dur, body_part bp = num_bp,
+        void add_effect( const efftype_id &eff_id, const time_duration &dur, body_part bp = num_bp,
                          bool permanent = false,
                          int intensity = 0, bool force = false, bool deferred = false ) override;
         /**
@@ -798,9 +798,9 @@ class Character : public Creature, public visitable<Character>
         /**Return list of available fuel for this bionic*/
         std::vector<itype_id> get_fuel_available( const bionic_id &bio ) const;
         /**Return available space to store specified fuel*/
-        int get_fuel_capacity( itype_id fuel ) const;
+        int get_fuel_capacity( const itype_id &fuel ) const;
         /**Return total space to store specified fuel*/
-        int get_total_fuel_capacity( itype_id fuel ) const;
+        int get_total_fuel_capacity( const itype_id &fuel ) const;
         /**Updates which bionic contain fuel and which is empty*/
         void update_fuel_storage( const itype_id &fuel );
         /**Get stat bonus from bionic*/
@@ -1264,7 +1264,7 @@ class Character : public Creature, public visitable<Character>
         }
         virtual bool query_yn( const std::string &msg ) const = 0;
 
-        bool is_immune_field( field_type_id fid ) const override;
+        bool is_immune_field( const field_type_id &fid ) const override;
         /** Returns true is the player is protected from electric shocks */
         bool is_elec_immune() const override;
         /** Returns true if the player is immune to this kind of effect */

src/clzones.cpp

@@ -561,7 +561,7 @@ void zone_manager::cache_vzones()
         const std::string &type_hash = elem->get_type_hash();
         auto &cache = area_cache[type_hash];
 
-        // @TODO: looks very similar to the above cache_data - maybe merge it?
+        // TODO: looks very similar to the above cache_data - maybe merge it?
 
         // Draw marked area
         for( const tripoint &p : tripoint_range( elem->get_start_point(), elem->get_end_point() ) ) {

src/construction.cpp

@@ -863,7 +863,7 @@ void complete_construction( player *p )
 
     award_xp( *p );
     // Friendly NPCs gain exp from assisting or watching...
-    // TODO NPCs watching other NPCs do stuff and learning from it
+    // TODO: NPCs watching other NPCs do stuff and learning from it
     if( p->is_player() ) {
         for( auto &elem : g->u.get_crafting_helpers() ) {
             if( elem->meets_skill_requirements( built ) ) {
@@ -1104,7 +1104,7 @@ void construct::done_deconstruct( const tripoint &p )
         }
         add_msg( _( "The %s is disassembled." ), f.name() );
         g->m.spawn_items( p, item_group::items_from( f.deconstruct.drop_group, calendar::turn ) );
-        // Hack alert.
+        // HACK: Hack alert.
         // Signs have cosmetics associated with them on the submap since
         // furniture can't store dynamic data to disk. To prevent writing
         // mysteriously appearing for a sign later built here, remove the

src/consumption.cpp

@@ -1210,7 +1210,7 @@ bool player::consume_effects( item &food )
             mod_fatigue( nutr );
         }
     }
-    // @TODO: remove this
+    // TODO: remove this
     int capacity = stomach_capacity();
     // Moved here and changed a bit - it was too complex
     // Incredibly minor stuff like this shouldn't require complexity
@@ -1253,7 +1253,7 @@ bool player::consume_effects( item &food )
 
     // Set up food for ingestion
     const item &contained_food = food.is_container() ? food.get_contained() : food;
-    // @TODO: Move quench values to mL and remove the magic number here
+    // TODO: Move quench values to mL and remove the magic number here
     units::volume water = contained_food.type->comestible->quench * 5_ml;
     food_summary ingested{
         water,

src/crash.cpp

@@ -23,7 +23,7 @@
 #endif
 
 #if defined(_WIN32)
-#if 1 // Hack to prevent reordering of #include "platform_win.h" by IWYU
+#if 1 // HACK: Hack to prevent reordering of #include "platform_win.h" by IWYU
 #include "platform_win.h"
 #endif
 #include <dbghelp.h>

src/creature.cpp

@@ -918,7 +918,7 @@ void Creature::add_effect( const effect &eff, bool force, bool deferred )
                 force, deferred );
 }
 
-void Creature::add_effect( const efftype_id &eff_id, const time_duration dur, body_part bp,
+void Creature::add_effect( const efftype_id &eff_id, const time_duration &dur, body_part bp,
                            bool permanent, int intensity, bool force, bool deferred )
 {
     // Check our innate immunity

src/creature.h

@@ -299,7 +299,7 @@ class Creature
         /** Returns true if we are immune to the field type with the given fid. Does not
          *  handle intensity, so this function should only be called through is_dangerous_field().
          */
-        virtual bool is_immune_field( const field_type_id ) const {
+        virtual bool is_immune_field( const field_type_id & ) const {
             return false;
         }
 
@@ -333,7 +333,7 @@ class Creature
         void add_effect( const effect &eff, bool force = false, bool deferred = false );
         /** Adds or modifies an effect. If intensity is given it will set the effect intensity
             to the given value, or as close as max_intensity values permit. */
-        virtual void add_effect( const efftype_id &eff_id, time_duration dur, body_part bp = num_bp,
+        virtual void add_effect( const efftype_id &eff_id, const time_duration &dur, body_part bp = num_bp,
                                  bool permanent = false, int intensity = 0, bool force = false, bool deferred = false );
         /** Gives chance to save via environmental resist, returns false if resistance was successful. */
         bool add_env_effect( const efftype_id &eff_id, body_part vector, int strength,

src/debug.cpp

@@ -40,7 +40,7 @@
 #endif
 
 #if defined(_WIN32)
-#   if 1 // Hack to prevent reordering of #include "platform_win.h" by IWYU
+#   if 1 // HACK: Hack to prevent reordering of #include "platform_win.h" by IWYU
 #       include "platform_win.h"
 #   endif
 #endif

src/debug_menu.cpp

@@ -1420,7 +1420,7 @@ void debug()
             g->display_toggle_overlay( ACTION_DISPLAY_RADIATION );
             break;
         case DEBUG_CHANGE_TIME: {
-            auto set_turn = [&]( const int initial, const time_duration factor, const char *const msg ) {
+            auto set_turn = [&]( const int initial, const time_duration & factor, const char *const msg ) {
                 const auto text = string_input_popup()
                                   .title( msg )
                                   .width( 20 )

src/dialogue.h

@@ -122,7 +122,7 @@ struct talk_effect_fun_t {
         void set_mapgen_update( const JsonObject &jo, const std::string &member );
         void set_bulk_trade_accept( bool is_trade, bool is_npc = false );
         void set_npc_gets_item( bool to_use );
-        void set_add_mission( std::string mission_id );
+        void set_add_mission( const std::string &mission_id );
         const std::vector<std::pair<int, std::string>> &get_likely_rewards() const;
         void set_u_buy_monster( const std::string &monster_type_id, int cost, int count, bool pacified,
                                 const translation &name );

src/editmap.cpp

@@ -1003,7 +1003,7 @@ void editmap::edit_feature()
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 ///// field edit
 
-void editmap::update_fmenu_entry( uilist &fmenu, field &field, const field_type_id idx )
+void editmap::update_fmenu_entry( uilist &fmenu, field &field, const field_type_id &idx )
 {
     int field_intensity = 1;
     const field_type &ftype = idx.obj();
@@ -1296,9 +1296,7 @@ void editmap::edit_itm()
     } while( ilmenu.ret != UILIST_CANCEL );
 }
 
-/*
- *  Todo
- */
+// TODO:
 void editmap::edit_critter( Creature &critter )
 {
     if( monster *const mon_ptr = dynamic_cast<monster *>( &critter ) ) {