[llvm-commits] [llvm] r119787 - in /llvm/trunk: include/llvm/ADT/IntervalMap.h lib/Support/CMakeLists.txt lib/Support/IntervalMap.cpp unittests/ADT/IntervalMapTest.cpp unittests/CMakeLists.txt

[Chris Lattner]

On Nov 18, 2010, at 8:47 PM, Jakob Stoklund Olesen wrote:

> Author: stoklund
> Date: Thu Nov 18 22:47:19 2010
> New Revision: 119787
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=119787&view=rev
> Log:
> Add ADT/IntervalMap.
> 
> This is a sorted interval map data structure for small keys and values with
> automatic coalescing and bidirectional iteration over coalesced intervals.

Very nice Jakob,

I like the libc++-style synopsis.  If this is a generally useful container, please add it to http://llvm.org/docs/ProgrammersManual.html#ds_map

> 
> +#include "llvm/Support/Allocator.h"
> +#include "llvm/Support/RecyclingAllocator.h"
> +#include <limits>
> +#include <iterator>
> +
> +// FIXME: Remove debugging code
> +#ifndef NDEBUG
> +#include "llvm/Support/raw_ostream.h"
> +#endif

Please don't do this though.  This means that we'll have compilation failures in disable-assertions mode because headers will accidentally depend on this header to export raw_ostream.  Either move the printing stuff to a .cpp file if possible, or unconditionally include raw_ostream.

+  void rmove(unsigned i, unsigned j, unsigned Count) {

When I first saw this, I thought it was a typo for 'remove'.  Would 'MoveRight' be better?  Likewise, xferLeft -> TransferLeft.

Overall, looks really nice!

-Chris