[Xapian-tickets] [Xapian] #830: std::string_view for functors

[Xapian]

#830: std::string_view for functors
--------------------------------+------------------------
        Reporter:  Olly Betts   |      Owner:  Olly Betts
            Type:  defect       |     Status:  new
        Priority:  normal       |  Milestone:  1.5.0
       Component:  Library API  |    Version:  git master
        Severity:  normal       |   Keywords:
      Blocked By:               |   Blocking:
Operating System:  All          |
--------------------------------+------------------------
 We now use `std::string_view` for most string parameters in the public
 API, but not for cases which are virtual methods for user subclassing.
 Changing these from `std::string` would require changes to user code, and
 doesn't seem to offer any immediate performance benefit - quoting from
 #785:

 > I noticed that for the functor virtual method case none of the existing
 places that call these functors inside the library would easily benefit
 from being able to pass std::string_view - they all take a std::string
 from somewhere else and pass it in to the functor. They also potentially
 actually make things less efficient - consider a user functor which wants
 a zero-byte terminated string to work on, which a std::string_view can't
 provide without the user making a std::string copy. It doesn't benefit the
 bindings either since in the functor case we're passing a string from C++
 to the target language (so much more like the situation with the return
 value in a normal method).
 >
 > There may be gains to be made here, but if there are they're going to
 need more widespread changes internally, so I'm thinking we should
 postpone this part for now.

 If we do change this, user code can do something like this to work with
 older and newer versions:

 {{{
 // XAPIAN_AT_LEAST was added in 1.4.2
 #if defined(XAPIAN_AT_LEAST) && XAPIAN_AT_LEAST(2.0.0)
 # define STRING_TYPE std::string_view
 #else
 # define STRING_TYPE const std::string&
 #endif

 // ...

 class MyExpandDecider : public Xapian::ExpandDecider {
   public:
     // Specify `override` to make sure we have the correct method
 signature.
     bool operator()(STRING_TYPE term) const override {
         // ...
     }
 };
 }}}

 For 1.5.0 we should update docs and examples to steer users towards
 specifying `override` on their overridden functor methods so they get a
 compile-time error if we later make this (or a different) change.  Without
 `override` you'll probably at least get a warning about the virtual
 function being hidden (`-Woverloaded-virtual` with GCC and clang).
-- 
Ticket URL: <https://trac.xapian.org/ticket/830>
Xapian <https://xapian.org/>
Xapian