#1511 Copyable entities and intersections
Metadata
| Property | Value |
|---|---|
| Reported by | Steffen Müthing (steffen.muething@iwr.uni-heidelberg.de) |
| Reported at | Oct 9, 2014 12:59 |
| Type | Discussion |
| Version | Git (pre2.4) [autotools] |
| Operating System | Unspecified / All |
| Last edited by | Steffen Müthing (steffen.muething@iwr.uni-heidelberg.de) |
| Last edited at | Feb 26, 2015 14:51 |
| Closed by | Steffen Müthing (steffen.muething@iwr.uni-heidelberg.de) |
| Closed at | Feb 26, 2015 14:51 |
| Closed in version | 2.4 |
| Resolution | Implemented |
| Comment |
Description
In Berlin, we decided to make entities and intersections copyable in 2.4. IIRC, that feature wasn't optional, and as it requires changes to both the grid interface and all grid implementations, I think we should really get to work on this...
Dominic will start on porting YaspGrid, but we have identified some points regarding the facade that aren't completely clear from the protocol of the developer meeting:
Constructors / Assignment Operators in the Facades
Do we want to be clever here and only enable the move variants if the grid explicitly enables movability? We'd probably need a capability for that. I'd say we don't bother - if you explicitly don't want to support movability, just delete the move versions with
Entity(Entity&&) = delete;
That syntax is supported by GCC 4.4 and up, and the move constructor of the facade class will automatically fall back to the copy constructor of the implementation (I verified that).
So I propose always providing public copy constructors, copy assignment operators, move constructors and move assignment operators for the Entity and Intersection facades, but allowing grid implementors to explicitly delete the move versions in the implementation classes.
Return Type when Dereferencing Iterators / EntityPointers
What should the user get when dereferencing an iterator? A const ref or a temporary? For intersections, the protocol says that starting from 3.0, the user will get a temporary, so for consistency, entities should work the same. For 2.4 I propose staying with const ref. Being clear about this is important because the choice has to be consistent across the facades and the implementations: If the facade returns a const ref, but the implementation returns a temporary, the user gets a const ref to an undefined memory location!
Default-Constructibility of Entities
When users can copy entities, they will start storing them in containers, which is easier if Entities are default-constructible. Should we support that? I don't like the idea, but maybe there are people out there with good use cases?
Until someone turns up with such a use case, I propose keeping the facade classes non default-constructible.
EntityPointer / Entity Interoperability
The whole EntityPointer deprecation and transition is still bothering me as well. As we want to get rid of the EntityPointer in 3.0, I think it would be really beneficial to have some kind of deprecation in place to help users locate all the places where they use one.
I had the following idea:
-
Return an Entity everywhere where we currently return an EntityPointer
-
Add deprecated operator* and operator-> methods to the Entity which return a copy (in the case of operator* - remember, we said we don't want to leak references) or a pointer to the Entity (in the case of operator->, where the pointer shouldn't hurt because you can't normally store it)
-
Add EntityPointer(Entity&&), operator=(const Entity&) and operator=(Entity&&) to make sure user code that expects to be handed an EntityPointer from the interface continues to work
-
Deprecate everything on the EntityPointer
I'm not sure if this will work correctly, but I think the only way to find out is by implementing it and testing it against existing code. Unless someone opposes this for something other than technical reasons (or comes up with a clear technical killer problem), I'd suggest implementing this for the facades and for YaspGrid (in a branch, of course) to make sure it works. This change would definitely need some kind of per-grid capability switch as well to avoid breaking grid implementations that aren't ported yet.
Discuss! ;-)
PS: We should really vote on each of those points later on, but I wanted to put the issues out there first, in particular the last one.