[Development] QList

[Thiago Macieira]

On quinta-feira, 23 de março de 2017 14:12:23 PDT Marc Mutz wrote:
> > I beg to differ. It *is* "is-a": any modifiable QString is also a view to
> > itself.
> 
> "is-a" is a terminus technicus. It means is-subtype-of aka.
> https://en.wikipedia.org/wiki/Liskov_substitution_principle
> 
> > Also, remember the point that it would be a private base, so the is-a
> > relationship is not visible. This is about code reuse, not about the
> > semantic.
> 
> If it's private inheritance, it's not is-a.

You're contradicting yourself now. If private inheritance is not is-a, then 
you can't tell me not to use private inheritance because QString isn't a 
QStringView.

I don't want QStringView to appear to users as a base of QString. I just want 
code reuse.

> > A naïve implementation of QString::mid() could be:
> > 
> > QString QString::mid(int start, int len) const
> > {
> > 
> > 	return QString(QStringView::mid(start, len));
> > 
> > }
> 
> Which is no different from
> 
>    return QString(QStringView(*this).mid(start, len));
> 
> so does not provide a case supporting using inhertance.

Actually, the code generation could be slightly different. If 
QStringView::mid() isn't inlined, then the difference is whether we need to 
create a temporary QStringView in the stack and pass its this pointer as a 
parameter.

> > > Last, there are no virtual functions in QStringView that QString would
> > > want to reimplement.
> > 
> > Virtual has nothing to do with anything. QString is complementing
> > QStringView, only adding functionality.
> 
> I'm merely listing the use-cases for inheritance. Good that you agree that
> none appy :)

I'm saying that virtual is not a requirement, therefore not having is not to 
the detriment of the solution.

> I said it many time, and I'll say it once more: The way to share code is
> through free functions, not one class delegating to another:

There's still a difference.

> 
>   return qFindChar(*this, ch, from, cs);
> 
> or
> 
>   return qFindString(*this, QStringView(&ch, 1), from, cs);
> 
> where only the qFoo() functions are exported, but none of the
> QString/View/Ref members.

I don't think we're going to go that far. Doing so would mean a lot of work to 
unexport the entire class and provide only free functions for the entirety of 
the QString/QStringView API.

Not to mention that we'd have source duplication, which is a source of both 
copy & paste errors and lack of copy & paste to add functions. I'd rather 
follow DRY.

> Note how all of those concerns are addressed by qCompareStrings()-like
> delegation to free functions.

Except for the concern that it adds of source code duplication.

> > Because MSVC ABI, like the Windows ABI, has severe shortcomings and
> > actually violate the C and C++ standards. Some of them are legacy reasons
> > from the 1980s, like the one causing crashes reported in QTBUG-38876
> > (attempt for Qt 6 fix at https://codereview.qt-project.org/189193 ).
> 
> I'm merely pointing out that inheriting one value class from another, even
> publicly (but the same problem would exist with private inheritance, I
> guess?), has already caused much pain. We should learn from it two things:
> 
> 1. not inhert one value type from another
> 2. not class-level-export value classes, only export out-of-line functions

I disagree with those learnings. The issue we have is deriving an exported 
class from a non-exported class, because suddenly all the inlines from the 
regular class become exported. It's a worse scenario when the base class was a 
template.

Deriving from an exported class has never been a problem.

As for doing (member) function-level exports, they're just ugly and violate 
DRY. We need very good reasons not to simply export the class itself.

Finally, exporting some free, low-level functions sounds like an acceptable 
idea. There's no reason not to have qstrlen and qstrchr operating on UTF-16, 
since neither the C or the C++ libraries offer that functionality. The only 
question is whether they should depend on QStringView or whether they should 
take the char16_t pointer and length.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center