[llvm] r243386 - Adding ADT SortedVector; client patch will follow.

[Puyan Lotfi]

> On Jul 27, 2015, at 11:50 PM, Justin Bogner <mail at justinbogner.com> wrote:
> 
> Puyan Lotfi <puyan at puyan.org> writes:
>> Author: zer0
>> Date: Tue Jul 28 01:04:00 2015
>> New Revision: 243386
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=243386&view=rev
>> Log:
>> Adding ADT SortedVector; client patch will follow.
>> 
>> Added:
>>    llvm/trunk/include/llvm/ADT/SortedVector.h
>> Modified:
>>    llvm/trunk/docs/ProgrammersManual.rst
>> 
>> Modified: llvm/trunk/docs/ProgrammersManual.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/ProgrammersManual.rst?rev=243386&r1=243385&r2=243386&view=diff
>> ==============================================================================
>> --- llvm/trunk/docs/ProgrammersManual.rst (original)
>> +++ llvm/trunk/docs/ProgrammersManual.rst Tue Jul 28 01:04:00 2015
>> @@ -1314,7 +1314,8 @@ never use hash_set and unordered_set bec
>> std::multiset is useful if you're not interested in elimination of duplicates,
>> but has all the drawbacks of :ref:`std::set <dss_set>`.  A sorted vector
>> (where you don't delete duplicate entries) or some other approach is almost
>> -always better.
>> +always better. LLVM actually offers SortedVector which does the job of a sorted
>> +std::vector.
> 
> Please elaborate a bit here. This doesn't really explain why you would
> choose SortedVector over a sorted std::vector. Similarly, sorted vectors
> aren't necessarily uniq'd, so it isn't really the same job.
> 

Agreed; its too short an explanation. The purpose of SortedVector is to serve a
usage pattern of appending to a vector repeatedly in a loop and sorting afterward.

The idea is that if you need a sorted vector of unique things you can use this ADT with less overhead
in certain situations without having to write all the same boilerplate you would if you used std::vector directly.

>> 
>> .. _ds_map:
>> 
>> 
>> Added: llvm/trunk/include/llvm/ADT/SortedVector.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SortedVector.h?rev=243386&view=auto
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ADT/SortedVector.h (added)
>> +++ llvm/trunk/include/llvm/ADT/SortedVector.h Tue Jul 28 01:04:00 2015
>> @@ -0,0 +1,133 @@
>> +//===-- llvm/ADT/SortedVector.h ---------------------------------*- C++ -*-===//
>> +//
>> +//                     The LLVM Compiler Infrastructure
>> +//
>> +// This file is distributed under the University of Illinois Open Source
>> +// License. See LICENSE.TXT for details.
>> +//
>> +//===----------------------------------------------------------------------===//
>> +
>> +#ifndef LLVM_ADT_SORTEDVECTOR_H
>> +#define LLVM_ADT_SORTEDVECTOR_H
>> +
>> +#include <vector>
>> +#include <cassert>
>> +#include <functional>
>> +#include "llvm/Support/raw_ostream.h"
>> +
>> +namespace llvm {
>> +
>> +/// \brief Lazily maintains a sorted and unique vector of elements of type T.
> 
> This description is a bit misleading. What this really does is enforce
> (through asserts) that you sort the vector before accessing it. When I
> see "lazily maintains" I get the impression that it will (somehow) do
> the sorting for me at the appropriate time.
> 


>> +template<typename T, typename CMP = std::less<T>>
>> +class SortedVector {
> 
> It might be a good idea to imply uniqueness in the name - sorted doesn't
> tell the whole story.
> 

I agree here but I felt that because we already have a UniqueVector, naming it SortedUniqueVector would be overly verbose. 


>> +public:
>> +  typedef typename std::vector<T> VectorType;
>> +  typedef typename VectorType::iterator iterator;
>> +  typedef typename VectorType::const_iterator const_iterator;
>> +
>> +private:
>> +  VectorType Vector;
>> +  bool IsSorted = true;
>> +
>> +  void doCheck() const {
> 
> I'd probably call this assertSorted.
> 

I’ll think about it; hmm.. sure why not.

>> +    assert(IsSorted && "Unsorted SortedVector access; call sortUnique prior.");
>> +  }
>> +
>> +public:
>> +  /// \brief Appends Entry to the sorted unique vector; sets the IsSorted flag
>> +  /// to false if appending Entry puts Vector into an unsorted state.
> 
> That we set the IsSorted flag is an implementation detail, better to
> describe this in terms of the user. Something like "Appends Entry. If
> this puts the vector into an unsorted state, sortUnique must be called
> before any accesses to the vector”.

I’ll change this comment too. 

> 

>> +  void insert(const T &Entry) {
>> +    if (!Vector.size())
>> +      Vector.push_back(Entry);
>> +
>> +    // Vector is sorted and Entry is a duplicate of the previous so skip.
>> +    if (IsSorted && Entry == Vector.back())
>> +      return;
>> +
>> +    IsSorted &= (CMP()(Vector.back(), Entry));
> 
> It seems a little odd to use std::less but to call operator== directly,
> doesn't it?
> 

Good catch. I’ll change this.

>> +    Vector.push_back(Entry);
>> +  }
>> +
>> +  // \brief Sorts and uniques Vector.
>> +  void sortUnique() {
>> +    if (IsSorted)
>> +      return;
>> +
>> +    std::sort(Vector.begin(), Vector.end());
>> +    Vector.erase(std::unique(Vector.begin(), Vector.end()), Vector.end());
>> +    IsSorted = true;
>> +  }
>> +
>> +  /// \brief Tells if Entry is in Vector without relying on sorted-uniqueness.
>> +  bool has(T Entry) const {
>> +    if (IsSorted)
>> +      return std::binary_search(Vector.begin(), Vector.end(), Entry);
>> +
>> +    return std::find(Vector.begin(), Vector.end(), Entry) != Vector.end();
>> +  }
>> +
>> +  /// \brief Returns a reference to the entry with the specified index.
>> +  const T &operator[](unsigned index) const {
>> +    assert(index < size() && "SortedVector index is out of range!");
>> +    doCheck();
>> +    return Vector[index];
>> +  }
>> +
>> +  /// \brief Return an iterator to the start of the vector.
>> +  iterator begin() {
>> +    doCheck();
>> +    return Vector.begin();
>> +  }
>> +
>> +  /// \brief Returns const iterator to the start of the vector.
>> +  const_iterator begin() const {
>> +    doCheck();
>> +    return Vector.begin();
>> +  }
>> +
>> +  /// \brief Returns iterator to the end of Vector.
>> +  iterator end() {
>> +    doCheck();
>> +    return Vector.end();
>> +  }
>> +
>> +  /// \brief Returns const iterator to the end of Vector. Assert if unsorted.
>> +  const_iterator end() const {
>> +    doCheck();
>> +    return Vector.end();
>> +  }
>> +
>> +  /// \brief Erases Vector at position. Asserts if Vector is unsorted.
>> +  iterator erase(iterator position) {
>> +    doCheck();
>> +    return Vector.erase(position);
>> +  }
>> +
>> +  /// \brief Erases Vector entirely.
>> +  iterator erase() {
>> +    IsSorted = true;
>> +    return Vector.erase();
>> +  }
>> +
>> +  /// \brief Returns number of entries in Vector; asserts if it is unsorted.
>> +  size_t size() const {
>> +    doCheck();
>> +    return Vector.size();
>> +  }
>> +
>> +  /// \brief Returns true if Vector is empty.
>> +  bool empty() const {
>> +    return Vector.empty();
>> +  }
>> +
>> +  /// \brief Clears all the entries.
>> +  void reset() {
>> +    IsSorted = true;
>> +    Vector.resize(0, 0);
>> +  }
>> +};
>> +
>> +} // End of namespace llvm
>> +
>> +#endif // LLVM_ADT_SORTEDVECTOR_H
>> +
>> 

Thanks

PL

>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits