Opened 5 years ago

Closed 4 years ago

#681 closed defect (fixed)

Make all functor objects non-copyable

Reported by: Olly Betts Owned by: Olly Betts
Priority: normal Milestone: 1.3.4
Component: Library API Version: SVN trunk
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Operating System: All

Description (last modified by Olly Betts)

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.

Change History (2)

comment:1 by Olly Betts, 5 years ago

Description: modified (diff)

comment:2 by Olly Betts, 4 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.