Re: [boost] Filesystem Library: Final version for Formal Review

From: Thomas Witt (witt_at_[hidden])
Date: 2002-09-14 17:01:13

On Saturday 14 September 2002 20:43, Beman Dawes wrote:
>
> I added a test case to operations_test.cpp, and it failed miserably at run
> time (protection fault) for all compilers except Borland:-(

To be honest I wasn't really expecting it to fail at runtime. This brings  me 
to the question what problem tried you to work around with reimplementing 
operator->?

>  Incrementing
> the iterator first in a separate expression, then using ->, worked
> correctly.

It should if it works for the non-incremented iterator.

>
> I frankly don't understand the details of this problem. Am I using
> iterator_adaptor incorrectly, or is there really a compiler problem, or is
> there a problem with iterator_adaptor?

Basically you are using an iterator class incorrectly. The rule to follow is : 
"Do not derive from an iterator, never.". For directory_iterator this leaves 
us with a problem with iterator_adaptor.

The problem with derivation is that the operator return types are incorrect 
for the derived class. This leads to all kind of surprising behaviour and may 
introduce hard to spot bugs.

E.g. (pseudo_code)

directory_iterator it(a_valid_path);
directory_iterator it1(it);     
directory_iterator it2(++it);  // Compile-time error
directory_iterator it1(it--); // Compile-time error

The problem is typeof(++it) != typeof(directory_iterator()).

For an iterator of derived type it-> calls derived::operator-> (++it)-> calls 
base::operator->.

Given the fact that derivation is a no-no, iterator_adaptor is not really well 
suited for directory_iterator. I only found this out while thinking about 
this posting, I should have realized this earlier. The problem is that no 
convenient and easy to use iterator ctor is provided. Having the user write 
something like directory_iterator(directory_iterator_internals(valid_path)) 
does not seem like an option to me.

To me there are two ways to solve the ctor problem in iterator_adaptor. The 
first would be to provide forwarding ctors in iterator adaptor. The second 
would be to use the barton-nackmann trick to adjust the operator return 
types. The first solution is far from complete given the current language. 
Think forwarding function problem or var arg templates.  In my opinion 
Barton-Nackmann would be a good solution if iterator_adaptor should be 
changed to support this use case.

Anyway all this are mid or long term options. For the short-term fix the 
following idea might do.

namespace boost {
  namespace filesystem {
    namespace detail {

      struct directory_iterator_internals
      {
        typedef boost::shared_ptr< detail::directory_iterator_imp > dii_ptr;
        dii_ptr imp;

        const path & deref() const;
        void inc();
      };

      struct dii_policies
      {

        intialize(path const& dir_path)
        {
          // initialize m_inter
          // Former directory_iterator ctor code goes here
        }

        template <class IteratorAdaptor>
        typename IteratorAdaptor::reference
        dereference(const IteratorAdaptor& x) const
          { return m_intern.deref(); }

        template <class IteratorAdaptor>
        void increment(IteratorAdaptor& x)
          { m_intern.inc(); }

        template <class IteratorAdaptor1, class IteratorAdaptor2>
        bool equal(const IteratorAdaptor1& x, const IteratorAdaptor2& y) const
          { return x.policies().m_intern.imp == y.policies().m_intern.imp; }

      private:
        directory_iterator_internals m_intern;
      };
    }  

    typedef iterator_adaptor<
      path,
      detail::dii_policies,
      path, const path &, const path *,
      std::input_iterator_tag, std::ptrdiff_t > directory_iterator;

  }
}

--Thomas

-- 
Dipl.-Ing. Thomas Witt
Institut fuer Verkehrswesen, Eisenbahnbau und -betrieb, Universitaet Hannover
voice: [phone] - 4273, fax: [phone]
http://www.ive.uni-hannover.de