[Development] Recommended way to take in strings

[A. Pönitz]

On Wed, May 31, 2023 at 07:17:21AM +0000, Marc Mutz via Development wrote:
> On 31.05.23 09:07, Sami Varanka via Development wrote:
> > What is the recommended way for functions to take strings? Our QtGraphs 
> > API takes in const QString & but is recommended way nowadays take in 
> > QAnyStringView?
> 
> I doubt there's an accepted project-wide standard, yet, but as a rule of 
> thumb that everyone might be able to agree on: If the function doesn't 
> store the string as-is (=parses or pre-processes it), take by 
> QAnyStringView, otherwise continue to take by QString cref.
> 
> For very important APIs, there's the option to overload on
> 
> - `const QString &` and/or `QString &&`
> - QAnyStringView
>
> In this case, the QString overloads have to be marked Q_WEAK_OVERLOAD to 
> avoid ambiguous calls with certain argument types (`const char*`, 
> `QByteArray`, etc).

Close, but I'd actually further qualify the precondition "If the function
doesn't store the string as-is" and possibly quantify the whole thing to


  "If the function doesn't *necessarily*
   store the string as-is (=parses or pre-processes it), take by 
   QAnyStringView, otherwise continue to take by QString cref.

   For *a rare, but* very important APIs, there's the option to overload on
 
     - `const QString &` and/or `QString &&`
     - QAnyStringView"


My reason for the "*necessarily*" is to not leak an incidental implementation
detail of a given implementation into the long-term API.

A "current" implementation for a certain function might get away with not
storing a copy, a possibly following improved version might actually _need_
a copy. There should be no restriction on future implementations of such
functions that might only be possible by storing a copy.


For the "rare": I'd like to re-iterate that introducing the /first/ overload
for /anything/ is source-incompatible. I was reminded /again/ of that after
updating to current Qt dev and finding out the hard way that some code that
compiled for ages (using "&QVariant::fromValue<int>") doesn't compile anymore
due to some change that introduced a generic r-value overload for it.

I will forgo any public pondering over (im-)possible setups where r-values for
int parameter have /any/ benefit and simply hope that this will fixed in
the actual 6.7 release, my point /here/ is that overloads should not be added
lightheartedly. Ever.


In the context of the original poster ("Qt Graph API") this means that since
any string-ish thing passed to that API will likely to be passed through
several more layers of API and finally be subject to some kind of optimized
delayed(!) rendering of the string, a generic benefit of having a QAnyStringView
is rather unlikely. /Optimal/ in that context is some ownership-transfering
mechanism taking input in the finally used encoding, second to that is anything
that created at least one deep copy. In that situation a non-overloaded 'const
QString &ref' parameter is simply "good enough".


Andre'