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

[Chad Rosier]

On Aug 6, 2011, at 10:45 PM, David Blaikie wrote:

>> 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?

Thanks for pointing this out; it is inefficient.

> 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;
>  }
> }

I committed the above in revision 137051.  The args list should be relatively short, so this solution is good.

 Regards,
  Chad

> 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