Hi, TL;DR: Never export non-polymorphic classes at class-level and always define destructors of polymorphic classes, and define them out-of-line.
The class-level export macro used to be a trigger for syncqt to generate a forwarding QClass header, so every public class had them in the past, and we have a rich history of exported classes. Over the years, however, we have had a variety of issues with such classes as opposed to those that export only member functions that are defined out-of-line and expected to be called from outside the module. An (incomplete) list of these issues can be found in TTLOFIR: https://wiki.qt.io/Things_To_Look_Out_For_In_Reviews#Value_Classes Item 6 These issues are subtle and many took _months_ to figure out. We have therefore begun, over time, to favour _not_ exporting non-polymorphic classes at class level in order to limit similar problems going forward. Syncqt has been fixed ages ago. Yes, it's more work to repeat the macro on most function instead of once at class level, but we trade more obvious typing against less subtle issues cropping up later. Minimizing typing for Qt class authors is not a design goal (and QtC's ALT-Down functionality for block editing works wonders for adding this). I've distilled this knowledge over time into the rule combination you see in the TL;DR: at the top of this email. This combination of rules has no False Positives that I am aware of. That means following these rules, your code is _always_ correct (within the scope of the rules). It's pretty rare to find rules in C++ that are that clear-cut. It's usually "prefer to" or "avoid', with tons of exceptions. Not here. There are no exceptions, and I think this will be one of the first Axivion checks we'd enable if and when we get to run it on the CI and report results in Gerrit. I'm happy to discuss alternatives with anyone who thinks we can relax that rule to continue to add non-polymorphic classes with class-level exports, but I expect more than "I don't like it". I expect a concrete alternative rule with a FP rate not higher than the one posted here. Note that I, at least, never saw most of the issues listed in TTLOFIR coming before they hit us. So it's also very hard to argue that some classes "never will run into this". We just don't know what else will crop up, until it does. So I maintain that the TL;DR: is the soundest engineering practice for exporting classes, and should be followed - without exception. Thanks, Marc PS: if you wonder about the second part of the rule, that's https://wiki.qt.io/Things_To_Look_Out_For_In_Reviews#Polymorphic_Classes Item 1 -- Marc Mutz <marc.m...@qt.io> (he/his) 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 -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
