Noissue - Use iterators syntax consistently

compiled/filter/ElemHideBase.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 115 matching lines @@
 
   return Type::ELEMHIDE;
 }
 
 OwnedString ElemHideBase::GetSelectorDomain() const
 {
   /* TODO this is inefficient */
   OwnedString result;
   if (mDomains)
   {
-    for (auto it = mDomains->begin(); it != mDomains->end(); ++it)
+    for (const auto& item : *mDomains)
     {
-      if (it->second && !it->first.empty())
+      if (item.second && !item.first.empty())
       {
         if (!result.empty())
           result.append(u',');
-        result.append(it->first);
+        result.append(item.first);
       }
     }

[Wladimir Palant, 2017/04/06 08:03:22]

Interestingly, this change resulted in less compact code for no obvious reason.
Code generated for other functions stayed identical.

[hub, 2017/04/06 08:55:27]

I'd be tempted to file a bug for that. How bad is it?

[Wladimir Palant, 2017/04/06 09:36:42]

Actually, I got it backwards it seems - the code got more compact now, 37 bytes
less than it was before. If you are really interested, the original loop was:

        do {
            if ((i[b + 8 >> 0] | 0) != 0 ? (o = b + 4 | 0, p = k[o >> 2] | 0, (p
& 1073741823 | 0) != 0) : 0) {
              ...
            }
            while (1) {
                c = b + 12 | 0;
                if ((c | 0) == (l | 0)) {
                    b = l;
                    break
                }
                if ((k[b + 16 >> 2] | 0) >>> 0 > 3221225471) b = c;
                else {
                    b = c;
                    break
                }
            }
            h = k[m >> 2] | 0
        } while ((b | 0) != ((k[h >> 2] | 0) + ((k[h + 4 >> 2] | 0) * 12 | 0) |
0));
        return;

And this is the new one:

        b: while (1) {
            if ((i[b + 8 >> 0] | 0) != 0 ? (n = b + 4 | 0, o = k[n >> 2] | 0, (o
& 1073741823 | 0) != 0) : 0) {
              ...
            }
            do {
                c = b;
                b = b + 12 | 0;
                if ((b | 0) == (l | 0)) {
                    b = 25;
                    break b
                }
            } while ((k[c + 16 >> 2] | 0) >>> 0 > 3221225471);
            if ((b | 0) == (l | 0)) {
                b = 25;
                break
            }
        }
        if ((b | 0) == 25) return

All in all not a huge difference, simply the optimizer taking a slightly
different approach when inlining HashContainerIterator::operator++. I do wonder
however where that 25 constant is coming from and what the point of it is - this
is end of function, so a conditional return makes no sense whatsoever at this
point. Never mind the fact that b will *always* be set to 25 when exiting this
loop.

   }
   return result;
 }