[Development] Using '#pragma once' instead of include guards?
Marc Mutz
marc.mutz at qt.io
Thu Mar 7 10:02:51 CET 2024
Hi Tor Arne,
On 06.03.24 09:34, Tor Arne Vestbø wrote:
> The choice of where we use/allow `#pragma once` should not be coupled to
> whether a header is considered public or not.
Let's be careful with wording here. No-one proposes to use #pragma once
to distinguish public from private headers!
The proposal is to distinguish installed from non-installed headers, and
the status quo is already that we can't have #pragma once in installed
headers.
> Let’s not create even more indirections and inference rules that one has
> to keep in mind.
>
> If we want to make it clearer if a header is public or not, let’s make
> it explicit.
>
> We already have two established mechanisms:
>
> - Header ending in _p.h
> - "We mean it” comment
These are for installed headers, and I'm actually proposing that the
rules don't change for _those_, only for uninstalled headers.
> Renaming files may be too much churn, so let’s add more "We mean it”
> comments.
>
> It doesn’t matter whether header will be installed or not.
I beg to differ. The whole topic came up because it _is_ currently
unclear whether a public-looking header is actually public (because it's
installed) or not (because it's not), and you have to look at the
location in the file system and/or the directory's CMakeLists.txt to
understand which one it is.
> The
> documentation is for whoever is going to look at the header, i.e us (and
> our tools) for non-installed headers, and for us and users for installed
> headers. Both groups would benefit from clarification of whether the
> header is public API or not.
If we were to add "We mean it" comments to uninstalled headers, don't
you think that would cause ambiguity in the other direction? Like "Oh,
this is a public header, why isn't it called _p.h"?
> I’m fine with using `#pragma once` more, but that discussion is
> orthogonal to how we clarify the status of a header.
It's not orthogonal for the reasons given in the first paragraph of this
email, but it also need not be parallel, indeed.
I just feel that the transition from a non-installed to an installed
header should leave some trace in header itself. #pragma once as a
static assertion that the header isn't installed means we need to port
to traditional header guards to make it installable, lest syncqt rejects it.
And, again: existing non-installed headers aside, which I mention I
don't propose to port, isn't it much simpler to write #pragma once than
to write a correct traditional header guard? So why wouldn't one use the
pragma for new headers? And if one would naturally do, why require to
add a (redundant) comment, too?
I feel that would be just more work for reviewers and authors. Any
header needs an inclusion guard, so one would suppose that authors and
reviewers already take care of that. The new thing is that installed
headers cannot use #pragma once, and that's enforced by tooling, so
can't be forgotten. Adding a comment to such headers is something
additional that authors and reviewers need to keep in mind.
I'd prefer the path of least resistance here.
Thanks,
Marc
> Cheers,
> Tor Arne
>
>> On 6 Mar 2024, at 07:57, Marc Mutz via Development
>> <development at qt-project.org> wrote:
>>
>> 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 at 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-bounces at qt-project.org> on behalf of
>>>> Marc Mutz via Development <development at qt-project.org>
>>>> *Sent:* Thursday, February 29, 2024 11:02
>>>> *To:* development at qt-project.org <development at 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>
>>>> <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.macieira at 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
>>>> <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 dittoforhttps://wiki.qt.io/Qt_Coding_Style
>>>> <forhttps://wiki.qt.io/Qt_Coding_Style>)
>>>> <https://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 at qt-project.org <mailto:Development at qt-project.org>
>>>>> https://lists.qt-project.org/listinfo/development
>>>>> <https://lists.qt-project.org/listinfo/development>
>>>> <https://lists.qt-project.org/listinfo/development
>>>> <https://lists.qt-project.org/listinfo/development>>
>>>> --
>>>> Marc Mutz <marc.mutz at qt.io <mailto:marc.mutz at qt.io>>
>>>> Principal Software Engineer
>>>>
>>>> The Qt Company
>>>> Erich-Thilo-Str. 10 12489
>>>> Berlin, Germany
>>>> www.qt.io <http://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 at qt-project.org <mailto:Development at qt-project.org>
>>>> https://lists.qt-project.org/listinfo/development
>>>> <https://lists.qt-project.org/listinfo/development>
>>>> <https://lists.qt-project.org/listinfo/development
>>>> <https://lists.qt-project.org/listinfo/development>>
>>>> --
>>>> Development mailing list
>>>> Development at qt-project.org
>>>> <mailto:Development at qt-project.org><mailto:Development at qt-project.org <mailto:Development at qt-project.org>>
>>>> https://lists.qt-project.org/listinfo/development
>>>> <https://lists.qt-project.org/listinfo/development>
>>>> <https://lists.qt-project.org/listinfo/development
>>>> <https://lists.qt-project.org/listinfo/development>>
>>>
>> --
>> Marc Mutz <marc.mutz at qt.io <mailto:marc.mutz at qt.io>>
>> Principal Software Engineer
>>
>> The Qt Company
>> Erich-Thilo-Str. 10 12489
>> Berlin, Germany
>> www.qt.io <http://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 at qt-project.org <mailto:Development at qt-project.org>
>> https://lists.qt-project.org/listinfo/development
>> <https://lists.qt-project.org/listinfo/development>
>
--
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
More information about the Development
mailing list