Skip to content

Commit

Permalink
Change i++ to ++i
Browse files Browse the repository at this point in the history
  • Loading branch information
David Jones authored and ShadowNinja committed Aug 25, 2015
1 parent 2480f2d commit 34b7a14
Show file tree
Hide file tree
Showing 41 changed files with 125 additions and 125 deletions.
12 changes: 6 additions & 6 deletions src/client.cpp
Expand Up @@ -86,7 +86,7 @@ MeshUpdateQueue::~MeshUpdateQueue()

for(std::vector<QueuedMeshUpdate*>::iterator
i = m_queue.begin();
i != m_queue.end(); i++)
i != m_queue.end(); ++i)
{
QueuedMeshUpdate *q = *i;
delete q;
Expand All @@ -113,7 +113,7 @@ void MeshUpdateQueue::addBlock(v3s16 p, MeshMakeData *data, bool ack_block_to_se
*/
for(std::vector<QueuedMeshUpdate*>::iterator
i = m_queue.begin();
i != m_queue.end(); i++)
i != m_queue.end(); ++i)
{
QueuedMeshUpdate *q = *i;
if(q->p == p)
Expand Down Expand Up @@ -146,7 +146,7 @@ QueuedMeshUpdate *MeshUpdateQueue::pop()
bool must_be_urgent = !m_urgents.empty();
for(std::vector<QueuedMeshUpdate*>::iterator
i = m_queue.begin();
i != m_queue.end(); i++)
i != m_queue.end(); ++i)
{
QueuedMeshUpdate *q = *i;
if(must_be_urgent && m_urgents.count(q->p) == 0)
Expand Down Expand Up @@ -617,7 +617,7 @@ void Client::step(float dtime)
{
for(std::map<int, u16>::iterator
i = m_sounds_to_objects.begin();
i != m_sounds_to_objects.end(); i++)
i != m_sounds_to_objects.end(); ++i)
{
int client_id = i->first;
u16 object_id = i->second;
Expand All @@ -642,7 +642,7 @@ void Client::step(float dtime)
i != m_sounds_server_to_client.end();) {
s32 server_id = i->first;
int client_id = i->second;
i++;
++i;
if(!m_sound->soundExists(client_id)) {
m_sounds_server_to_client.erase(server_id);
m_sounds_client_to_server.erase(client_id);
Expand Down Expand Up @@ -1105,7 +1105,7 @@ void Client::sendRemovedSounds(std::vector<s32> &soundList)
pkt << (u16) (server_ids & 0xFFFF);

for(std::vector<s32>::iterator i = soundList.begin();
i != soundList.end(); i++)
i != soundList.end(); ++i)
pkt << *i;

Send(&pkt);
Expand Down
6 changes: 3 additions & 3 deletions src/client/tile.cpp
Expand Up @@ -194,7 +194,7 @@ class SourceImageCache
public:
~SourceImageCache() {
for (std::map<std::string, video::IImage*>::iterator iter = m_images.begin();
iter != m_images.end(); iter++) {
iter != m_images.end(); ++iter) {
iter->second->drop();
}
m_images.clear();
Expand Down Expand Up @@ -461,7 +461,7 @@ TextureSource::~TextureSource()

for (std::vector<TextureInfo>::iterator iter =
m_textureinfo_cache.begin();
iter != m_textureinfo_cache.end(); iter++)
iter != m_textureinfo_cache.end(); ++iter)
{
//cleanup texture
if (iter->texture)
Expand All @@ -471,7 +471,7 @@ TextureSource::~TextureSource()

for (std::vector<video::ITexture*>::iterator iter =
m_texture_trash.begin(); iter != m_texture_trash.end();
iter++) {
++iter) {
video::ITexture *t = *iter;

//cleanup trashed texture
Expand Down
2 changes: 1 addition & 1 deletion src/clientmap.cpp
Expand Up @@ -228,7 +228,7 @@ void ClientMap::updateDrawList(video::IVideoDriver* driver)
u32 sector_blocks_drawn = 0;

for(MapBlockVect::iterator i = sectorblocks.begin();
i != sectorblocks.end(); i++)
i != sectorblocks.end(); ++i)
{
MapBlock *block = *i;

Expand Down
6 changes: 3 additions & 3 deletions src/collision.cpp
Expand Up @@ -176,7 +176,7 @@ bool wouldCollideWithCeiling(

for(std::vector<aabb3f>::const_iterator
i = staticboxes.begin();
i != staticboxes.end(); i++)
i != staticboxes.end(); ++i)
{
const aabb3f& staticbox = *i;
if((movingbox.MaxEdge.Y - d <= staticbox.MinEdge.Y) &&
Expand Down Expand Up @@ -265,7 +265,7 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
std::vector<aabb3f> nodeboxes = n.getCollisionBoxes(gamedef->ndef());
for(std::vector<aabb3f>::iterator
i = nodeboxes.begin();
i != nodeboxes.end(); i++)
i != nodeboxes.end(); ++i)
{
aabb3f box = *i;
box.MinEdge += v3f(x, y, z)*BS;
Expand Down Expand Up @@ -320,7 +320,7 @@ collisionMoveResult collisionMoveSimple(Environment *env, IGameDef *gamedef,
f32 distance = speed_f.getLength();
std::vector<u16> s_objects;
s_env->getObjectsInsideRadius(s_objects, pos_f, distance * 1.5);
for (std::vector<u16>::iterator iter = s_objects.begin(); iter != s_objects.end(); iter++) {
for (std::vector<u16>::iterator iter = s_objects.begin(); iter != s_objects.end(); ++iter) {
ServerActiveObject *current = s_env->getActiveObject(*iter);
if ((self == 0) || (self != current)) {
objects.push_back((ActiveObject*)current);
Expand Down
2 changes: 1 addition & 1 deletion src/content_cao.cpp
Expand Up @@ -1820,7 +1820,7 @@ std::string GenericCAO::debugInfoText()
os<<"GenericCAO hp="<<m_hp<<"\n";
os<<"armor={";
for(ItemGroupList::const_iterator i = m_armor_groups.begin();
i != m_armor_groups.end(); i++)
i != m_armor_groups.end(); ++i)
{
os<<i->first<<"="<<i->second<<", ";
}
Expand Down
4 changes: 2 additions & 2 deletions src/content_mapblock.cpp
Expand Up @@ -1548,7 +1548,7 @@ void mapblock_mesh_generate_special(MeshMakeData *data,
std::vector<aabb3f> boxes = n.getNodeBoxes(nodedef);
for(std::vector<aabb3f>::iterator
i = boxes.begin();
i != boxes.end(); i++)
i != boxes.end(); ++i)
{
for(int j = 0; j < 6; j++)
{
Expand Down Expand Up @@ -1689,7 +1689,7 @@ void mapblock_mesh_generate_special(MeshMakeData *data,
v3f pos = intToFloat(p, BS);
f32 d = 0.05 * BS;
for (std::vector<aabb3f>::iterator i = boxes.begin();
i != boxes.end(); i++) {
i != boxes.end(); ++i) {
aabb3f box = *i;
box.MinEdge += v3f(-d, -d, -d) + pos;
box.MaxEdge += v3f(d, d, d) + pos;
Expand Down
8 changes: 4 additions & 4 deletions src/craftdef.cpp
Expand Up @@ -964,10 +964,10 @@ class CCraftDefManager: public IWritableCraftDefManager
{
std::ostringstream os(std::ios::binary);
os << "Crafting definitions:\n";
for (int type = 0; type <= craft_hash_type_max; type++) {
for (int type = 0; type <= craft_hash_type_max; ++type) {
for (std::map<u64, std::vector<CraftDefinition*> >::const_iterator
it = (m_craft_defs[type]).begin();
it != (m_craft_defs[type]).end(); it++) {
it != (m_craft_defs[type]).end(); ++it) {
for (std::vector<CraftDefinition*>::size_type i = 0;
i < it->second.size(); i++) {
os << "type " << type
Expand All @@ -992,10 +992,10 @@ class CCraftDefManager: public IWritableCraftDefManager
}
virtual void clear()
{
for (int type = 0; type <= craft_hash_type_max; type++) {
for (int type = 0; type <= craft_hash_type_max; ++type) {
for (std::map<u64, std::vector<CraftDefinition*> >::iterator
it = m_craft_defs[type].begin();
it != m_craft_defs[type].end(); it++) {
it != m_craft_defs[type].end(); ++it) {
for (std::vector<CraftDefinition*>::iterator
iit = it->second.begin();
iit != it->second.end(); ++iit) {
Expand Down
12 changes: 6 additions & 6 deletions src/environment.cpp
Expand Up @@ -613,19 +613,19 @@ class ABMHandler
= abm->getRequiredNeighbors();
for(std::set<std::string>::iterator
i = required_neighbors_s.begin();
i != required_neighbors_s.end(); i++)
i != required_neighbors_s.end(); ++i)
{
ndef->getIds(*i, aabm.required_neighbors);
}
// Trigger contents
std::set<std::string> contents_s = abm->getTriggerContents();
for(std::set<std::string>::iterator
i = contents_s.begin(); i != contents_s.end(); i++)
i = contents_s.begin(); i != contents_s.end(); ++i)
{
std::set<content_t> ids;
ndef->getIds(*i, ids);
for(std::set<content_t>::const_iterator k = ids.begin();
k != ids.end(); k++)
k != ids.end(); ++k)
{
content_t c = *k;
std::map<content_t, std::vector<ActiveABM> >::iterator j;
Expand Down Expand Up @@ -694,7 +694,7 @@ class ABMHandler
continue;

for(std::vector<ActiveABM>::iterator
i = j->second.begin(); i != j->second.end(); i++) {
i = j->second.begin(); i != j->second.end(); ++i) {
if(myrand() % i->chance != 0)
continue;

Expand Down Expand Up @@ -772,7 +772,7 @@ void ServerEnvironment::activateBlock(MapBlock *block, u32 additional_dtime)
MapNode n;
for(std::map<v3s16, NodeTimer>::iterator
i = elapsed_timers.begin();
i != elapsed_timers.end(); i++){
i != elapsed_timers.end(); ++i){
n = block->getNodeNoEx(i->first);
v3s16 p = i->first + block->getPosRelative();
if(m_script->node_on_timer(p,n,i->second.elapsed))
Expand Down Expand Up @@ -1161,7 +1161,7 @@ void ServerEnvironment::step(float dtime)
MapNode n;
for(std::map<v3s16, NodeTimer>::iterator
i = elapsed_timers.begin();
i != elapsed_timers.end(); i++){
i != elapsed_timers.end(); ++i){
n = block->getNodeNoEx(i->first);
p = i->first + block->getPosRelative();
if(m_script->node_on_timer(p,n,i->second.elapsed))
Expand Down
8 changes: 4 additions & 4 deletions src/event_manager.h
Expand Up @@ -53,7 +53,7 @@ class EventManager: public MtEventManager
if(i != m_dest.end()){
std::list<FuncSpec> &funcs = i->second.funcs;
for(std::list<FuncSpec>::iterator i = funcs.begin();
i != funcs.end(); i++){
i != funcs.end(); ++i){
(*(i->f))(e, i->d);
}
}
Expand Down Expand Up @@ -83,20 +83,20 @@ class EventManager: public MtEventManager
if(remove)
funcs.erase(j++);
else
j++;
++j;
}
}
} else{
for(std::map<std::string, Dest>::iterator
i = m_dest.begin(); i != m_dest.end(); i++){
i = m_dest.begin(); i != m_dest.end(); ++i){
std::list<FuncSpec> &funcs = i->second.funcs;
std::list<FuncSpec>::iterator j = funcs.begin();
while(j != funcs.end()){
bool remove = (j->f == f && (!data || j->d == data));
if(remove)
funcs.erase(j++);
else
j++;
++j;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/fontengine.cpp
Expand Up @@ -122,7 +122,7 @@ void FontEngine::cleanCache()

for (std::map<unsigned int, irr::gui::IGUIFont*>::iterator iter
= m_font_cache[i].begin();
iter != m_font_cache[i].end(); iter++) {
iter != m_font_cache[i].end(); ++iter) {
iter->second->drop();
iter->second = NULL;
}
Expand Down
14 changes: 7 additions & 7 deletions src/game.cpp
Expand Up @@ -375,7 +375,7 @@ PointedThing getPointedThing(Client *client, v3f player_position,

for (std::vector<aabb3f>::const_iterator
i = boxes.begin();
i != boxes.end(); i++) {
i != boxes.end(); ++i) {
aabb3f box = *i;
box.MinEdge += npf;
box.MaxEdge += npf;
Expand Down Expand Up @@ -420,7 +420,7 @@ PointedThing getPointedThing(Client *client, v3f player_position,
if (!g_settings->getBool("enable_node_highlighting")) {
for (std::vector<aabb3f>::const_iterator
i2 = boxes.begin();
i2 != boxes.end(); i2++) {
i2 != boxes.end(); ++i2) {
aabb3f box = *i2;
box.MinEdge += npf + v3f(-d, -d, -d) - intToFloat(camera_offset, BS);
box.MaxEdge += npf + v3f(d, d, d) - intToFloat(camera_offset, BS);
Expand Down Expand Up @@ -514,11 +514,11 @@ class ProfilerGraph
std::map<std::string, Meta> m_meta;

for (std::vector<Piece>::const_iterator k = m_log.begin();
k != m_log.end(); k++) {
k != m_log.end(); ++k) {
const Piece &piece = *k;

for (Profiler::GraphValues::const_iterator i = piece.values.begin();
i != piece.values.end(); i++) {
i != piece.values.end(); ++i) {
const std::string &id = i->first;
const float &value = i->second;
std::map<std::string, Meta>::iterator j =
Expand Down Expand Up @@ -550,7 +550,7 @@ class ProfilerGraph
u32 next_color_i = 0;

for (std::map<std::string, Meta>::iterator i = m_meta.begin();
i != m_meta.end(); i++) {
i != m_meta.end(); ++i) {
Meta &meta = i->second;
video::SColor color(255, 200, 200, 200);

Expand All @@ -566,7 +566,7 @@ class ProfilerGraph
s32 meta_i = 0;

for (std::map<std::string, Meta>::const_iterator i = m_meta.begin();
i != m_meta.end(); i++) {
i != m_meta.end(); ++i) {
const std::string &id = i->first;
const Meta &meta = i->second;
s32 x = x_left;
Expand Down Expand Up @@ -602,7 +602,7 @@ class ProfilerGraph
bool lastscaledvalue_exists = false;

for (std::vector<Piece>::const_iterator j = m_log.begin();
j != m_log.end(); j++) {
j != m_log.end(); ++j) {
const Piece &piece = *j;
float value = 0;
bool value_exists = false;
Expand Down
2 changes: 1 addition & 1 deletion src/genericobject.cpp
Expand Up @@ -110,7 +110,7 @@ std::string gob_cmd_update_armor_groups(const ItemGroupList &armor_groups)
writeU8(os, GENERIC_CMD_UPDATE_ARMOR_GROUPS);
writeU16(os, armor_groups.size());
for(ItemGroupList::const_iterator i = armor_groups.begin();
i != armor_groups.end(); i++){
i != armor_groups.end(); ++i){
os<<serializeString(i->first);
writeS16(os, i->second);
}
Expand Down
8 changes: 4 additions & 4 deletions src/guiFormSpecMenu.cpp
Expand Up @@ -2103,7 +2103,7 @@ bool GUIFormSpecMenu::getAndroidUIInput()
}

for(std::vector<FieldSpec>::iterator iter = m_fields.begin();
iter != m_fields.end(); iter++) {
iter != m_fields.end(); ++iter) {

if (iter->fname != fieldname) {
continue;
Expand Down Expand Up @@ -2473,7 +2473,7 @@ void GUIFormSpecMenu::drawMenu()

if (id != -1 && delta >= m_tooltip_show_delay) {
for(std::vector<FieldSpec>::iterator iter = m_fields.begin();
iter != m_fields.end(); iter++) {
iter != m_fields.end(); ++iter) {
if ( (iter->fid == id) && (m_tooltips[iter->fname].tooltip != "") ){
if (m_old_tooltip != m_tooltips[iter->fname].tooltip) {
m_old_tooltip = m_tooltips[iter->fname].tooltip;
Expand Down Expand Up @@ -3609,7 +3609,7 @@ bool GUIFormSpecMenu::OnEvent(const SEvent& event)
std::string GUIFormSpecMenu::getNameByID(s32 id)
{
for(std::vector<FieldSpec>::iterator iter = m_fields.begin();
iter != m_fields.end(); iter++) {
iter != m_fields.end(); ++iter) {
if (iter->fid == id) {
return iter->fname;
}
Expand All @@ -3625,7 +3625,7 @@ std::string GUIFormSpecMenu::getNameByID(s32 id)
std::wstring GUIFormSpecMenu::getLabelByID(s32 id)
{
for(std::vector<FieldSpec>::iterator iter = m_fields.begin();
iter != m_fields.end(); iter++) {
iter != m_fields.end(); ++iter) {
if (iter->fid == id) {
return iter->flabel;
}
Expand Down
2 changes: 1 addition & 1 deletion src/guiKeyChangeMenu.cpp
Expand Up @@ -81,7 +81,7 @@ GUIKeyChangeMenu::~GUIKeyChangeMenu()
removeChildren();

for (std::vector<key_setting*>::iterator iter = key_settings.begin();
iter != key_settings.end(); iter ++) {
iter != key_settings.end(); ++iter) {
delete[] (*iter)->button_name;
delete (*iter);
}
Expand Down
4 changes: 2 additions & 2 deletions src/guiscalingfilter.cpp
Expand Up @@ -51,13 +51,13 @@ void guiScalingCache(io::path key, video::IVideoDriver *driver, video::IImage *v
void guiScalingCacheClear(video::IVideoDriver *driver)
{
for (std::map<io::path, video::IImage *>::iterator it = g_imgCache.begin();
it != g_imgCache.end(); it++) {
it != g_imgCache.end(); ++it) {
if (it->second != NULL)
it->second->drop();
}
g_imgCache.clear();
for (std::map<io::path, video::ITexture *>::iterator it = g_txrCache.begin();
it != g_txrCache.end(); it++) {
it != g_txrCache.end(); ++it) {
if (it->second != NULL)
driver->removeTexture(it->second);
}
Expand Down
2 changes: 1 addition & 1 deletion src/hud.cpp
Expand Up @@ -468,7 +468,7 @@ void Hud::drawCrosshair() {
void Hud::drawSelectionBoxes(std::vector<aabb3f> &hilightboxes) {
for (std::vector<aabb3f>::const_iterator
i = hilightboxes.begin();
i != hilightboxes.end(); i++) {
i != hilightboxes.end(); ++i) {
driver->draw3DBox(*i, selectionbox_argb);
}
}
Expand Down

12 comments on commit 34b7a14

@roflo1
Copy link

@roflo1 roflo1 commented on 34b7a14 Aug 26, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really a thing?

@PilzAdam
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roflo1 yes, the postfix increment operator is slower for non-primitive types.

@sfan5
Copy link
Member

@sfan5 sfan5 commented on 34b7a14 Aug 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PilzAdam I just tested this and it is completly wrong, as long as you tell the compiler to optimize a little it will even generate the exactly identical assembly code

http://irc.minetest.ru/minetest-dev/2015-08-28#i_4379979

@bluebear94
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We mustn't rely too much on compiler optimizations. I propose keeping it as ++i.

@PilzAdam
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfan5 debug builds hate you!

@asl97
Copy link
Contributor

@asl97 asl97 commented on 34b7a14 Aug 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we just merge it into the comparison? eg: iter++ != m_buttons[i].ids.end(), would that work (better)?

The postfix increment ++ does not increase the value of its operand until after it has been evaluated.

the only reason i could think of not to do it is that it could be confusing for people who don't know about it.

@sfan5
Copy link
Member

@sfan5 sfan5 commented on 34b7a14 Aug 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PilzAdam look further below, debugging works equally good with -O1 and tools like valgrind do not require -O0 either

@bluebear94 might aswell re-write Minetest in assembler since compilers are completly incompetent, right?

@nerzhul
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfan5 i agree with the second part... compiler does their work, debug builds don't need perf improvements...

@HybridDog
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compiler does their work, debug builds don't need perf improvements

Does luajit do such work, too?

@sfan5
Copy link
Member

@sfan5 sfan5 commented on 34b7a14 Sep 3, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compiler does their work, debug builds don't need perf improvements

Does luajit do such work, too?

Yes, LuaJIT optimizes Lua code too. However I don't get how this is relevant here.

@HybridDog
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However I don't get how this is relevant here.

l guess it takes a little bit time when something gets compiled just in time before it's used. Maybe changing i++ to ++i takes a little bit time from compiling minetest

@sfan5
Copy link
Member

@sfan5 sfan5 commented on 34b7a14 Sep 3, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, but LuaJIT works with Lua code.
This commit doesn't even touch any Lua code.

Please sign in to comment.