[Development] On Removing Public Undocumented/\internal APIs

[Marc Mutz]

TL;DR: Make qdoc openly document \internal members of documented 
classes/namespaces or defined in public headers as "Internal. Subject to 
change without notice. If you feel this function should be public, file 
a bug report with your use-case."?

On 07.03.24 10:09, Volker Hilsheimer wrote:
>> On 4 Mar 2024, at 10:57, Marc Mutz via Development <development at qt-project.org> wrote:
>>
>> Hi,
>>
>> TL;DR:
>> - Treat all APIs not clearly marked as private (private access, *Private
>>    namespace or "We mean it!" comment) as public, in particular keep SC/BC
>>    and deprecate before remove.
>> - Avoid adding APIs that aren't clearly private or public in the future.
> 
> […]
> 
>> I would suggest that we treat any entity defined in a header file that
>> is not in a *Private namespace or under the umbrella of an "We mean it!"
>> commant, in short: anything that isn't _visible_ as non-public API _from
>> the header file_, as semi-public, and apply the same rules as for public
>> API: keep BC/SC, deprecate first, remove second, and adjust QUIP-6
>> accordingly.
> 
> I think we have to acknowledge that Qt is 30 years old this year, and over the decades the capabilities of tools, the features of the languages, and the best practices, skills, and awareness of the people working on Qt have changed a lot. What usually hasn’t changed is the *intent* of the class author - even though it might be buried deep in the historical repositories. Stating now that everything that has the technical appearance of a public API cannot be changed, and ignoring the author’s intent, does not seem constructive.

Two users have come out and said that they think public undocumented API 
is fair game. If there's a difference between the expectations of users 
and expectations of API authors, do you _really- propose to pick the 
author's?

> 
>> ¹ Off the top of my head: QString::isSimpleText(), QPen::data_ptr(),
>> QDeferredDeleteEvent
> 
> Undocumented APIs in any SDK are, IMHO, “use at your own risk”. I (or my IDE) might find something promising in some public header, but if I have no information about what the author of that thing is promising me, should I use it in my code? Sometimes it’s more obvious than other times, but what on earth is the semantics of "QString::isSimpleText”? What is that “DataPtr” reference returned by "QPen::data_ptr”? Can anyone use QDeferredDeleteEvent::loopLevel() in a meaningful way without studying the event delivery code?

It's not for me to prove that these functions are useful or not. It's 
for the changers and removers of these functions to prove that no-one 
uses them.

But ok, let's see:

- the data_ptr() could be used as a null-check (there's no QPen::isNull())
- QDeferredDeleteEvent could have been posted to an object in another 
thread, because there was doubt as to whether deleteLater() was actually 
thread-safe, but posting events is known to be (_I_ had to look that up 
myself just now).

I have no use-case for isSimpleText(), but what, exactly, did the Qt 
project gain by removing it instead of deprecating it as "We have found 
no use for this function. If you use it, please report a bug."?

The question that remains unanswered is this: Given that _we_ know how 
to deal with change and _we_ control the changelog, why should our 
_users_ be subjected to unannounced breaks of in-good-faith code?

There's a time for such changes: a major release. A minor release is - 
IMHO - not a time for such changes.

We have all the tools we need to not step on our users' toes here, while 
still pre-programming for a smooth transition for us come Qt 7. If we 
decide to nevertheless do these changes before Qt 7, what, exactly, does 
that say about our priorities?

>> At the same time, we should make sure that we don't accept such
>> semi-public APIs going forward anymore.
> 
> I do agree that we shouldn’t add more such stuff, at least not without making it visible in the header, ideally in a way that makes it visible to code completion (if that’s possible at all).
> 
> For stuff that is there today (e.g. various container types’ “isSharedWith” public member function, usually documented explicitly as \internal, and often used in other modules or in tests so not really practical to do it any other way), should we add a "// internal” comment in the header file?

Why is isSharedWith() \internal in the first place? The containers are 
publicly documented to be implicitly shared, so the likes of isShared(), 
isSharedWith() and detach() should be public documented functions, IMHO.

For new \internal API, something like _internal as part of the function 
name is probably a good idea. Then qdoc could automatically skip them. 
Oh, that gives me an idea: QDoc could take \internal members of a 
documented class/namespace (or defined in a public header) and 
explicitly document them as "Internal, subject to change w/o notice. If 
you feel this entity should be public API, report a bug <link>.". That 
has the advantage that it solves the problem for all existing functions, 
in one fell swoop, even for past Qt versions. Give it an LTS cycle to 
sink in, and then we grant ourselves the freedom to act on it again. 
Even so: deprecating instead of removing should remain the gold 
standard. But QPen::data_ptr() would have been covered by this, at least.

Thanks,
Marc

-- 
Marc Mutz <marc.mutz at qt.io>
Principal Software Engineer

The Qt Company
Erich-Thilo-Str. 10 12489
Berlin, Germany
www.qt.io

Geschäftsführer: Mika Pälsi, Juha Varelius, Jouni Lintunen
Sitz der Gesellschaft: Berlin,
Registergericht: Amtsgericht Charlottenburg,
HRB 144331 B