[Dune] Returning Geometries As Objects

[Martin Nolte]

Hi Steffen,

what you say is true: Such code might break in a very subtle way. Unfortunately, 
the compiler seems not to be able to warn when caching such a reference to a 
temporary. However, I think this problem will only occur in discretization 
frameworks, and, hence is under our direct control. In dune-fem, geometry 
references were only stored in very few places.

The issue concerning meta grids is not really a problem. Only very few people 
have ever created their own grid and they shouldn't have any problem adapting 
their code. Moreover, I'm very willing to help in case of problems. I would also 
like to point out that, having already changed such a meta grid (IdGrid), the 
code looks much nicer and clearer, now. And for the 2.5 guys who actually care: 
It is also faster.

Best,

Martin


On 02/08/2012 07:05 PM, Steffen Müthing wrote:
> Hi all,
>
> Am 25.01.2012 um 09:49 schrieb Martin Nolte:
>
>> Dear all,
>>
>> as announced in December, there is now an implementation returning the geometry as an object and all grid implementations in dune-grid have been adapted accordingly. Therefore, I think time has come for discussion of the outcome.
>>
>
>>
>> As already mentioned in the announcement of the transition, there is also a subtle impact on user code. Geometry references may no longer be stored outside the scope calling the geometry method. For example:
>>
>> const Geometry *pgeo = 0;
>> {
>>   const Geometry&geo = entity.geometry();
>>   pgeo =&geo;
>> }
>>
>> worked with the current interface but will break after the transition. Notice that the compiler will now warn in such a situation and the resulting code will probably segfault. The good news is that the segfault is extremely likely (since, e.g., pgeo now points onto the execution stack). However, such code will be rare and probably only found in the higher discretization modules, such as dune-fem, dune-fufem, or dune-pdelab.
>
>
> I just realized that this problem can also come up when the reference is a
> class member, as might happen in a caching structure of a discretization
> module (like the first argument passed to the local operator in PDELab),
> and any code with a cache object like this:
>
> template<typename GV>
> class Cache
> {
>
>   Cache(const Entity&  e)
>     : geometry(e.geometry())
>   {}
>
>   const Geometry&  geometry;
>
> };
>
> will probably break, as the compiler will destroy the temporary returned by
> the call to Entity::geometry() as soon as the constructor exits (cf. section
> 12.2.5 of the standard). As Martin pointed out, the referenced address is
> on the stack and will probably be overwritten pretty quickly, but I'm not sure
> whether that will always cause a direct segfault or instead taunt us with subtle
> and hard-to-track bugs. And unfortunately, we don't get a compiler warning
> in this case (at least not with plain -Wall).
>
> In PDELab we do not run into this problem right now because we only cache
> the entity and not the geometry, but I don't know about the other assembler
> frameworks out there.
>
> The second area where this issue might show up is in metagrids that simply
> reuse the underlying geometry - in that case, the Geomety wrapper needs to
> somehow store the geometry, and as it can only do that by reference or using
> a pointer, all metagrid maintainers should probably check whether their grid
> works using Martin's branch (I admit, I haven't done any checking on
> MultiDomainGrid yet, but I am pretty sure that my grid will have to be modified
> to work with this change - will do some testing tomorrow).
>
> Even if we decide that the issue is not a real problem for the geometries, I'm afraid
> that it might cause a lot of major breakage in user code once we start the conversion
> to value-typed Entities (I do hope most everyone caches those, or all of our courses
> have been in vain ;-) ).
>
> As I said, I don't know how dangerous this is in reality (especially for the geometries),
> but I thought I'd better bring it up...
>
>
> Steffen
>
> PS: I attached a very simple example illustrating the issue.
>
>
>
>
>
>
>
> Steffen Müthing
> Universität Stuttgart
> Institut für Parallele und Verteilte Systeme
> Universitätsstr. 38
> 70569 Stuttgart
> Tel: +49 711 685 88429
> Fax: +49 711 685 88340
> Email: steffen.muething at ipvs.uni-stuttgart.de
>
>
>
>
>
>
> _______________________________________________
> Dune mailing list
> Dune at dune-project.org
> http://lists.dune-project.org/mailman/listinfo/dune

-- 
Dr. Martin Nolte <nolte at mathematik.uni-freiburg.de>

Universität Freiburg                                   phone: +49-761-203-5630
Abteilung für angewandte Mathematik                    fax:   +49-761-203-5632
Hermann-Herder-Straße 10
79104 Freiburg, Germany