[Dune] [Dune-Commit] dune-grid r7145 - trunk/dune/grid/common

[Carsten Gräser]

Hi,
for me removing constructor and assignement is fine. The only problem is
that it might be used out there (with other grids). From our point of
view it's a bug, from the users point of view it's a feature (that is
buggy in GeometryGrid).

Since we cannot fix the bug while beeing compatible I'd favor Roberts
suggestion to enable it with a copatibility macro. According to the
decision for FieldVector::size we should use: DUNE_GRID_RELEASE_COMPATIBILITY

Best,
Carsten


Am 07.01.2011 14:40, schrieb Martin Nolte:
> Hi Oli,
> 
> no offense taken. But discussing everything in the FlySpray for two weeks will result in such things
> never being fixed (like my little patch to the MCMGMapper ;-)). And, as I said, errors resulting
> from this "misuse" are very hard to track.
> 
> Best,
> 
> Martin
> 
> On 01/07/2011 02:34 PM, Oliver Sander wrote:
>> Hi Robert, hi Martin!
>> I agree with you that the public operators were bugs that needed fixing.
>> But I disagree when you say that fixing a bug in the interface is not an
>> interface change.
>> This discussion is not academic, because we actually do use the assignment
>> operator in our code. Is it anywhere to be seen that we were not supposed to?
>> Hence your change broke our code, without a prior warning.
>>
>> Don't get me wrong: I am not complaining, just trying to inform you about
>> the background of Carsten's mail.
>>
>> Best,
>> Oliver
>>
>> Am 07.01.2011 14:25, schrieb Martin Nolte:
>>> Hi Carsten,
>>>
>>> yes, it is very late. And, as Robert stated, I don't consider this an
>>> interface change. The reason for changing is a "bug" in the dune-grid-howto
>>> code:
>>>
>>> const Intersection i = *intersectionIterator;
>>>
>>> This failed to work with GeometryGrid. So I had to decide: Is this an error
>>> in GeometryGrid or is this kind of use disallowed. My understanding of the
>>> interface is the second and, since this "bug" was hard to find, I changed
>>> the interface implementation accordingly.
>>>
>>> The final discussion should also depend on the outcome of the "Remove
>>> EntityPointer" suggestion, which was postponed until the next developer
>>> meeting. So, I suggust to postpone the issue until then.
>>>
>>> Finally, in my optionion, such discussions belong into the FlySpray to
>>> simplify voting (and my vote here is obvious).
>>>
>>> Best,
>>>
>>> Martin
>>>
>>>
>>> On 01/07/2011 11:12 AM, Carsten Gräser wrote:
>>>> Hi,
>>>> altough this comment is a little late:
>>>>
>>>> Was there any discussion on this interface change?
>>>> Even if we agree to forbid the copy constructor
>>>> and assignment they should at least remain public
>>>> for the next release.
>>>>
>>>> In case we want to remove them: Is anyone aware of
>>>> a mechanism that generates warnings only if it's
>>>> used in a public context?
>>>>
>>>> Best,
>>>> Carsten
>>>>
>>>>
>>>> Am 27.11.2010 13:56, schrieb mnolte at dune-project.org:
>>>>> Author: mnolte
>>>>> Date: 2010-11-27 13:56:58 +0100 (Sat, 27 Nov 2010)
>>>>> New Revision: 7145
>>>>>
>>>>> Modified:
>>>>> trunk/dune/grid/common/intersection.hh
>>>>> Log:
>>>>> make copy constructor protected. Intersections, like entities, may not be
>>>>> copied or assigned to
>>>>>
>>>>>
>>>>> Modified: trunk/dune/grid/common/intersection.hh
>>>>> ===================================================================
>>>>> --- trunk/dune/grid/common/intersection.hh 2010-11-27 12:45:59 UTC (rev 7144)
>>>>> +++ trunk/dune/grid/common/intersection.hh 2010-11-27 12:56:58 UTC (rev 7145)
>>>>> @@ -407,10 +407,6 @@
>>>>> /** Copy Constructor from IntersectionImp */
>>>>> Intersection(const IntersectionImp<const GridImp> & i) :
>>>>> real(i) {};
>>>>> -
>>>>> - /** Copy constructor */
>>>>> - Intersection(const Intersection& i) :
>>>>> - real(i.real) {}
>>>>> //@}
>>>>>
>>>>> typedef typename remove_const<GridImp>::type mutableGridImp;
>>>>> @@ -430,6 +426,17 @@
>>>>> //! return reference to the real implementation
>>>>> const ImplementationType& getRealImp() const { return real; }
>>>>>
>>>>> + /* hide copy constructor */
>>>>> + Intersection ( const Intersection&i )
>>>>> + : real( i.real )
>>>>> + {}
>>>>> +
>>>>> + /* hide assignment operator */
>>>>> + const Intersection&operator= ( const Intersection&i )
>>>>> + {
>>>>> + real = i.real;
>>>>> + return *this;
>>>>> + }
>>>>> };