[Lldb-commits] [lldb] r129595 - /lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp

[Stephen Wilson]

Hi Johnny,

On Fri, Apr 15, 2011 at 07:56:24PM -0000, Johnny Chen wrote:
> +    // Remove the merged ranges by shifting down all the keepers...
> +    std::set<size_t> purged(indices.begin(), indices.end());
> +    size_t new_size = m_aranges.size() - indices.size();
> +    for (size_t src = 0, dst = 0; dst < new_size; ++dst)
> +    {
> +        while (purged.count(src) > 0)
> +            ++src;
> +        if (src == dst)
> +            continue;
> +        m_aranges[dst] = m_aranges[src];
> +    }

That looks broken to me.  Index zero is never in the purged set since we
merge ranges into their predecessors.  Therefore purged.count(0) == 0
and on the first iteration we have src == dst, so we increment dst.
What ends up happening is that src remains zero and we just do:

    for (i = 0; i < size; ++i) { m_arages[i] = m_aranges[0]; }


Now, I *think* I know what you were trying to do here, and I think the
main motivation on your side was to "clean up the code" -- but since the
name of the game of the patch was optimization replacing the original
O(n) approach with a "clean"  O(n log(n)) implementation is not the way
to go IMHO.

Is there something I can do to clean up the original patch without
reducing it's performance?


Cheers,

-- 
steve