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

[Oliver Sander]

Hi there,
I have no objection to removing code that was deprecated before the 2.1 
release.
Even this close to the next branching.
best,
Oliver

Am 27.04.2012 14:41, schrieb Martin Nolte:
> Hi Carsten,
>
> I have no objections to removing it (it is deprecated and I never got
> the warning even before the 2.1 release).
>
> Best,
>
> Martin
>
> On 04/27/2012 02:33 PM, Carsten Gräser wrote:
>> 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).
>>>>
>>>>
>>>> _______________________________________________
>>>> Dune mailing list
>>>> Dune at dune-project.org
>>>> http://lists.dune-project.org/mailman/listinfo/dune
>