[cfe-commits] [PATCH] Revised patch for generating diagnostics information for crashing clang

David Blaikie dblaikie at gmail.com
Sat Aug 6 22:45:54 PDT 2011


> Attached is a revised patch for generating diagnostics information when clang crashes.  Would someone mind giving it a quick review? In particular, I'm interested in feedback on the cmake configuration as this caused buildbot failures in the past.  I was able to build and test using cmake on my Darwin system.

Just curious:

+void ArgList::eraseArg(OptSpecifier Id) {
+  for (iterator it = begin(), ie = end(); it != ie; ++it) {
+    if ((*it)->getOption().matches(Id)) {
+      Args.erase(it);
+      it = begin();
+      ie = end();
+    }
+  }
+}
+

This seems inefficient - do you need to restart from the beginning each time?

Unless there's something non-obvious (which could be commented, if
that's the case) you could use the iterator returned by erase:

for (iterator it = begin(), ie = end(); it != ie; ) {
  if ((*it)->getOption().matches(Id)) {
    it = Args.erase(it);
  } else {
    ++it;
  }
}

but, better than that - this is actually O(N^2) still, since erase is
linear - you'd want to use the erase-remove idiom (
http://en.wikipedia.org/wiki/Erase-remove_idiom ) but that might not
be worth implementing, since it's a bit more work for probably very
small sets in this case::

iterator iw = begin();
for (iterator ir = begin(), ie = end(); ir != ie; ++ir) {
  if (!(*it)->getOption().matches(Id)) {
    // probably cheaper just to assign the pointers than check whether
iw == ir, so just copy regardless
    *iw = *ir;
    ++iw;
  }
}
Args.erase(iw, end());

perhaps?

- David




More information about the cfe-commits mailing list