Hi, TL;DR: I'd maintain that #pragma once to mark non-installed headers is a) the least-intrusive change of all that's been proposed, b) targeting the non-public headers only (so don't disturb e.g. the API reviews (or customer's validation...)), which c) are the minority of Qt headers (minimizing work) and d) has benefits other than as a static assertion against header installation.
Longer version: Given that there are, in general, many more public and _p.h private headers in Qt (outside tools), I'd think the prudent approach is to modify the (fewer) non-installed headers. As for using a comment: We have the "We mean it" comment in _installed_ non-public headers, because installed headers are _accessible_ to users. I see no value in adding such comments to headers that are never installed, and are therefore _in_accessible to users. OTOH, #pragma once has benefits other than acting as a static assertion against header installation. In my forays into "leaf" modules, but even in qtbase, misnamed (header guard name doesn't match file name) and non-Q-namespaced or overly-generic header guard macro names abound, and I even found a downright broken header guard that had only #ifndef/#endif and was missing the #define. If we can make it easier for Qt devs to protect headers, we should do it. #pragma once also doesn't change when the file is renamed or otherwise moved. Note to self: make sanity bot check that header guards start with a Q and end in the file's name xor contain #pragma once, except in /3rdparty/. Thanks, Marc On 05.03.24 11:43, Volker Hilsheimer wrote: >> On 4 Mar 2024, at 15:56, Kai Köhne via Development >> <development@qt-project.org> wrote: >> >> Hi Marc, >> >> I've nothing against using '#pragma once' for private/internal headers. >> >> But you said you mainly want to have this to differentiate between >> different types of headers. If this is the motivation, I think we can >> make this differentiation even more explicit. For instance, public >> headers could get a >> >> // This header is part of the public Qt API. >> >> comment. Much like the 'We mean it', or 'pragma once', syncqt could >> enforce this for public headers, and error out if it's used for >> non-public ones. >> >> Kai > > > I think the challenge then is again how syncqt can know what a public > header is. How does syncqt know that src/plugins/**/*.h headers are not > public headers? They look like public headers, except for the “plugins” > in the path. How do we, on a build system level, distinguish between > “private installed” and “private non-installed” headers? > > In the end, syncqt can ideally rely on an explicit decision that has > become manifest through an easily recognizable pattern in each header > file. Whether we replace include guards with #pragma in all non-public > headers, or tag all public headers with a comment doesn’t really matter > all that much, does it? > > But given that we have the “We mean it” comment already for _p.h > headers, would it not be more consistent if we simply add that comment > to all non-public headers (no matter their file path, and no matter > whether the header is installed or not)? That comment makes the > usability of the declarations in the header obvious to the reader, > without having to know the rules. > > We have agreed that for some headers we allow use of #pragma, but taking > myself as a reference, I doubt that it’s obvious to everyone which > headers are installed, and when it’s allowed to use #pragma, and when > it’s mandatory to use #pragma. Perhaps adding the “We mean it” comment > to all headers not declaring public API is less obscure? The question is > if and how we can use syncqt to enforce this reliably. > > > Volker > > > >> ------------------------------------------------------------------------ >> *From:* Development <development-boun...@qt-project.org> on behalf of >> Marc Mutz via Development <development@qt-project.org> >> *Sent:* Thursday, February 29, 2024 11:02 >> *To:* development@qt-project.org <development@qt-project.org> >> *Subject:* Re: [Development] Using '#pragma once' instead of include >> guards? >> Hi, >> >> DL;DR: Use #pragma once in all non-installed headers >> >> The question recently came up "what is a private header". And the answer >> isn't just "_p.h, of course". We have tons of headers that are "private" >> without being marked as such with _p.h and "We mean it." comment. >> >> The first realization is that there are degrees of privateness: We have >> the installed private headers, and then we have non-installed/able >> headers, e.g. in plugins, or tools. >> >> So we have >> - public installed headers (subject to SC and BC, syncqt and >> headerscheck runs on them) >> - semi-public installed headers (like above, but not subject to SC (but >> BC) (_impl.h, stuff in QtPrivate namespaces, qNN, ...) >> - private installed headers (not subject to SC/BC/headersclean, but >> syncqt runs on them, must have "We mean it." comment) >> - private non-installed headers (not subject to any constraint, not even >> syncqt runs on them) >> >> We can now look at what signs we currently have available that guide a >> reader to learn which kind of header he's looking at. >> >> For the first, we have only location in $SRCDIR. >> >> For the second, we have _impl.h and/or "We mean it." comment. >> >> For the third, which is easiest to distinguish, we have _p.h and "We >> mean it." comment. This is enforced by syncqt, which is why we can rely >> on it 100%. >> >> For the last one, we again have just the location in $SRCDIR. >> >> The problem is, obviously, that the first and last cases are nearly >> indistinguishable and require non-local reasoning to answer. >> >> I think we have improve on this. >> >> With Volker's email we gave ourselves permission to use #pragma once for >> "non-SDK" (= non-installed) headers, and banned it for installed >> headers. So if we could make syncqt complain if a processed (= >> installable) header contains #praga once, we could then flip the coin >> and use an actual #pragma once as a static assertion that the header is >> not installed/able. >> >> If we do this going forward, we can then easily distinguish the four >> header kinds: >> >> - public installed headers have a traditional header guard >> - semi-public installed headers ditto, except that have _impl.h suffix >> or "We mean it" comment >> - private installed headers ditto, _p.h suffix and "We mean it" comment >> - non-installed/able headers have #pragma once >> >> I've implemented the check in syncqt.cpp and ported xcb over, see >> https://codereview.qt-project.org/q/topic:pragma-once >> <https://codereview.qt-project.org/q/topic:pragma-once> >> >> I'm not suggesting to do such a port for all plugins. XCB is just a test >> balloon, but we might want to apply the #pragma once trick for new code >> going forward. >> >> Thanks, >> Marc >> >> On 12.10.22 12:35, Volker Hilsheimer via Development wrote: >> > >> >> On 11 Oct 2022, at 22:11, Thiago Macieira <thiago.macie...@intel.com> >> >> wrote: >> >> >> >> On Tuesday, 11 October 2022 12:25:13 PDT Kyle Edwards via Development >> >> wrote: >> >>> Speaking as co-maintainer of CMake, we have effectively required #pragma >> >>> once to build CMake itself since August 2017, we officially codified >> >>> this as policy in September 2020, and we will soon be writing a >> >>> clang-tidy plugin to enforce this in our CI. We have not received any >> >>> complaints about it. Just my $0.02. >> >> >> >> Thanks for the information. This confirms what we already knew that all >> >> systems >> >> and compilers where Qt would be compiled do support it. >> >> >> >> However, neither Qt Creator nor CMake are libraries. They are not >> >> comparable. >> > >> > >> > Thanks all for sharing your insights and digging up the previous >> > discussions as well. >> > >> > The summary of all this then seems to be: >> > >> > - ok to use '#pragma once’ in headers that are not designed to be included >> > by Qt users, i.e. in tools, applications, examples and demos, tests >> > - for everything else, in particular for public and, for consistency’s >> > sake - private headers in Qt, we continue to use conventional include >> > guards >> > >> > Rationale: #pragma once is not well enough defined and not part of the >> > standard, and we cannot make any assumptions about how Qt is installed, >> > used as part of a larger SDK etc. So best to stay conservative. >> > >> > If that’s not entirely off, then I’d like to put this >> > intohttps://wiki.qt.io/Coding_Conventions >> <https://wiki.qt.io/Coding_Conventions> [1], preempting perhaps a new >> thread on this topic in a few years. >> > >> > Volker >> > >> > [1]: And since that page seems rather outdated - e.g. we do use >> > dynamic_cast in Qt today, and the suggestion to normalize signals and >> > slots should rather suggest to make connections via PMF syntax - perhaps >> > it’s time to move this to a QUIP where we can discuss and review such >> > changes in gerrit. I won’t have time to do that for a >> while (perhaps ditto forhttps://wiki.qt.io/Qt_Coding_Style) >> <https://wiki.qt.io/Qt_Coding_Style)>, but perhaps someone else wants >> to give this a shot. >> > >> > _______________________________________________ >> > Development mailing list >> > Development@qt-project.org >> >https://lists.qt-project.org/listinfo/development >> <https://lists.qt-project.org/listinfo/development> >> -- >> Marc Mutz <marc.m...@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 >> >> -- >> Development mailing list >> Development@qt-project.org >> https://lists.qt-project.org/listinfo/development >> <https://lists.qt-project.org/listinfo/development> >> -- >> Development mailing list >> Development@qt-project.org <mailto:Development@qt-project.org> >> https://lists.qt-project.org/listinfo/development >> <https://lists.qt-project.org/listinfo/development> > -- Marc Mutz <marc.m...@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 -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
