Opened 7 months ago
Last modified 7 months ago
#830 new defect
std::string_view for functors
Reported by: | Olly Betts | Owned by: | Olly Betts |
---|---|---|---|
Priority: | normal | Milestone: | 2.0.0 |
Component: | Library API | Version: | git master |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Operating System: | All |
Description
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).
d508bea1dedb2327c6ebe1327599c5db6ac97388 adds
override
to (I think) all the API functor subclasses in the codebase. I've also added them to the getting started guide.The rest isn't 1.5.0 material.