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

[Puyan Lotfi]

Ping

On Wed, Jul 29, 2015 at 5:40 PM, Puyan Lotfi <plotfi at apple.com> wrote:

> Justin, Aaron:
>
> These are the revised patches.
> I have tried to address all of your feedback with the exception of the
> naming of the ADT.
> I was thinking, if I did rename it I could go with SortedVectorSet or
> SortedSet.
>
> Anyways, I have added:
>
> - Better documentation changes that try to be more clear on how and when
> to use SortedVector.
> - Better comments.
> - Removed erase().
> - Replaced reset with clear.
> - Renamed insert to push_back
> - Added unit testing.
>
> Hope this is enough to put my patch back in; please let me know if I
> missed anything.
>
> Thanks
>
> PL
>
>
>
>
>
>
> > 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.
> >
> >>
> >> .. _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.
> >
> >> +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.
> >
> >> +    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".
> >
> >> +  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?
> >
> >> +    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
> >> +
> >>
> >>
> >> _______________________________________________
> >> 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150731/29546595/attachment.html>