[Dune] dune-grid does not compile when implementations of EntityPointer and Iterator are not interoperable

[Carsten Gräser]

Hi,
Am 27.04.2012 13:00, schrieb Martin Nolte:
> Hi Steffen,
> 
> you are right, the cast operator is indeed deprecated in the 2.1 release
> branch. This would mean it should be gone by now. Unfortunately, we are
> close to a release and I would ask the release managers whether such a
> change would still be in order. Is anybody still using the deprecated
> cast operator?
in my opinion we should really remove it. And I don't expect any
problems in the core modules.

> For the casting problem, you might want to have a look at GeometryGrid's
> entity pointer (see dune/grid/geometrygrid/entitypointer.hh). Here, the
> entity pointer stores the host iterator (which could, e.g., be a leaf
> iterator). You can then derive your iterator from the entity pointer
> (with the correct iterator passed as template argument) and, thus,
> satisfy the interoperability desired by Dune::EntityPointer. Of course,
> this trick has some consequences:
> (a) There is no guarantee that the code will work
>     (but that holds for the cast in Dune::EntityPointer as well).
> (b) The last member of your entity pointer must be the host iterator
>     (which is the one actually casted by the reinterpret_cast).
> Apart from that, the code seems to work wonderfully. I have to admit,
> though, that I never use the deprecated cast operator in my code.
> 
> Best,
> 
> Martin
> 
> On 04/27/2012 12:08 PM, Steffen Müthing wrote:
>> Hi Dune,
>>
>> I just noticed that it is not possible to have non-interoperable
>> implementations of EntityPointer and any
>> of the iterators - doing so will make the compiler choke on the
>> deprecated EntityPointer reference cast.
>>
>> I stumbled over the problem when I forcibly disabled the
>> interoperability (by specializing EnableIfInteroperable)
>> to debug an issue in MultiDomainGrid. I was able to make fix the issue
>> (see attached patch), but from the wording
>> in entitypointer.hh, I really don't know whether or not the
>> compilation is expected to fail for non-interoperable
>> implementations (as some kind of compile-time safeguard).
I suppose it was intended to be a compile-time safeguard.
However, it does not really provide any safety because
the interoperability check works fine if you can convert
in any direction. IMHO one would need to use enable_if with

  IsBaseOf<EntityPointerImp,IteratorImp>

instead of IsInteroperable to check if the reinterpret_cast
could do something meaningful (at least if you disregard the
standard). Even then one would need to apply your trick to
make the code compile if the latter is not true.

>>
>> That said, being able to disable the possibility to do the reference
>> cast would really simplify the MultiDomainGrid
>> code quite a bit in a number of places - right now I have the problem
>> that the cast matches rather greedily and I have to
>> provide pretty elaborate template specializations of constructors and
>> other stuff to work around that.
>>
>> So, is this intended behavior or should we fix it? Or do we throw out
>> the reference cast for the new release (it was deprecated
>> in 2.1)?
It's not intended but I suggest to remove the cast anyway. If there are
any objections I'd fix the problem as you suggested and be more strict
for the cast to be enabled (see attached patch).

Best,
Carsten

>>
>> Best,
>>
>> Steffen
>>
>>
>> PS: In order to fix the problem, I had to do some trickery with the
>> cast operator - it can't be disabled using SFINAE, because the return
>> type *is* the function name. So I did a less than pretty hack and just
>> made it convert to void if the cast is disabled (a marker type like
>> InvalidCast or something might be nicer).
-------------- next part --------------
Index: entitypointer.hh
===================================================================
--- entitypointer.hh	(Revision 8004)
+++ entitypointer.hh	(Arbeitskopie)
@@ -118,10 +118,13 @@
 protected:
   Implementation realIterator;
 
-  // autocheck whether imp is convertable into imp::base
-  typedef typename
-  Dune::EnableIfInterOperable<typename IteratorImp::EntityPointerImp,IteratorImp,
-                              typename IteratorImp::EntityPointerImp>::type base;
+  // select EntityPointer with const base class of implementation
+  // as engine only if it is the base class
+  // use void as dummy otherwise
+  typedef typename SelectType<
+     Dune::IsBaseOf< typename IteratorImp::EntityPointerImp, IteratorImp >::value,
+     EntityPointer<GridImp,typename IteratorImp::EntityPointerImp>&,
+     void >::Type EntityPointerCastTargetReference;
 
 public:
   //! codimension of entity pointer 
@@ -183,9 +186,9 @@
    *  \note  This cast is no longer needed to ensure assignability of iterators to
    *         EntityPointer.
    */
-  operator const EntityPointer< GridImp, base > & () const DUNE_DEPRECATED
+  operator const EntityPointerCastTargetReference () const DUNE_DEPRECATED
   {
-    return reinterpret_cast<const EntityPointer<GridImp,base>&>(*this);
+    return reinterpret_cast<const EntityPointerCastTargetReference>(*this);
   }
 
   //@}