[Xapian-tickets] [Xapian] #681: Make all functor objects non-copyable

[Xapian]

#681: Make all functor objects non-copyable
-------------------------+------------------------------
 Reporter:  olly         |             Owner:  olly
     Type:  defect       |            Status:  new
 Priority:  normal       |         Milestone:  1.3.4
Component:  Library API  |           Version:  SVN trunk
 Severity:  normal       |        Resolution:
 Keywords:               |        Blocked By:
 Blocking:               |  Operating System:  All
-------------------------+------------------------------
Description changed by olly:

Old description:

> I noticed that some of the Xapian classes which exist to be subclassed to
> implement custom functionality explicitly prevent copying and assignment
> (via private copy ctor and assignment operator):
>
>  * `ErrorHandler`
>  * `MatchSpy`
>  * `PostingSource`
>  * `Weight`
>
> Others currently permit copying and assignment:
>
>  * `ExpandDecider`
>  * `FieldProcessor` (new in 1.3.x)
>  * `KeyMaker`
>  * `MatchDecider`
>  * `StemImplementation`
>  * `Stopper`
>  * `ValueRangeProcessor`
>
> I think it's a mistake that we allow this.
>
> You can only correctly copy an object of a known subclass - if you try to
> copy an object via a pointer or reference to the base class, either it
> will only copy the base class (slicing the object), or if the base class
> is abstract it will fail to compile.
>
> User code which is making use of this is presumably doing something like
> passing by value instead of by reference, or keeping a vector of these
> objects which copies them when the internal storage is reallocated (a
> vector of pointers would be a better approach - even if copying the
> subclass is cheap, `vector<FooKeyMaker>` can only hold objects which are
> exactly of class `FooKeyMaker`, whereas `vector<KeyMaker*>` or
> vector<unique_ptr<KeyMaker>>` can hold an object of any subclass of
> `KeyMaker`).
>
> Unless there's a situation in which this copying or assignment is an
> appropriate way to achieve something, I propose we simply disable this in
> 1.3.x, rather than having the usual deprecation cycle - it's not quite "a
> feature which [doesn't] actually work", but to me it looks like one which
> is sufficiently problematic that you wouldn't knowingly use it.

New description:

 I noticed that some of the Xapian classes which exist to be subclassed to
 implement custom functionality explicitly prevent copying and assignment
 (via private copy ctor and assignment operator):

  * `ErrorHandler`
  * `MatchSpy`
  * `PostingSource`
  * `Weight`

 Others currently permit copying and assignment:

  * `ExpandDecider`
  * `FieldProcessor` (new in 1.3.x)
  * `KeyMaker`
  * `MatchDecider`
  * `StemImplementation`
  * `Stopper`
  * `ValueRangeProcessor`

 I think it's a mistake that we allow this.

 You can only correctly copy an object of a known subclass - if you try to
 copy an object via a pointer or reference to the base class, either it
 will only copy the base class (slicing the object), or if the base class
 is abstract it will fail to compile.

 User code which is making use of this is presumably doing something like
 passing by value instead of by reference, or keeping a vector of these
 objects which copies them when the internal storage is reallocated (a
 vector of pointers would be a better approach - even if copying the
 subclass is cheap, `vector<FooKeyMaker>` can only hold objects which are
 exactly of class `FooKeyMaker`, whereas `vector<KeyMaker*>` or
 `vector<unique_ptr<KeyMaker>>` can hold an object of any subclass of
 `KeyMaker`).

 Unless there's a situation in which this copying or assignment is an
 appropriate way to achieve something, I propose we simply disable this in
 1.3.x, rather than having the usual deprecation cycle - it's not quite "a
 feature which [doesn't] actually work", but to me it looks like one which
 is sufficiently problematic that you wouldn't knowingly use it.

--

--
Ticket URL: <http://trac.xapian.org/ticket/681#comment:1>
Xapian <http://xapian.org/>
Xapian