[coreboot] Re: [RFC] #pragma once
Thanks for digging up all that useful information, Martin! So `#pragma once` is not the clear winner after all. Too bad since we could eliminate some boilerplate code with that approach. Is there another way to solve the problem Arthur raised in this thread? The LMKL thread had a python script and people were also talking about adding something to checkpatch.pl. Maybe these days it's sufficient to assume clang will catch any real problems (CB:62173). On Mon, May 16, 2022 at 1:03 PM ron minnich wrote: > we have, in the past, used Linux kernel style as our guideline on > coreboot style. > > I'd say, based on Martin's note, that #pragma once is not such a good > idea after all. If we decide NOT to use it, however, let's put a note > about it in our style guide? > > This is not the first time this question has come up. > > > On Mon, May 16, 2022 at 12:34 PM Martin Roth > wrote: > > > > After reading what I've included below, I'm going to have to vote to > keep the guards. > > Martin > > > > May 16, 2022, 10:59 by david.hendri...@gmail.com: > > > > > On Mon, May 16, 2022 at 8:59 AM ron minnich > wrote: > > > > > >> > > >> btw, sometimes this has gone the other direction .. > > >> https://github.com/lowRISC/opentitan/pull/5693 > > >> > > > > > > It looks like they did that solely to conform to Google's style guide > > > which, dogmatic as it may appear, makes sense since OpenTitan is a > > > Google-lead project. > > > > > The question then is 'why does Google require the use of guards?'. > Whatever you think of google, they're not going to mandate something like > this without a good reason. > > > > I went searching for where this rule came from, and found this: > > > > ``` > > If you trust our in-house C++ compiler gurus, here's the most salient > part of the whole thread linked above.Matt Austern (4/11/2013): If you talk > to the authors of [most C++] compilers, I think you'll find that most of > them consider "#pragma once" (or the equivalent #import) to be a flaky > feature -- something that works almost all of the time and that can cause > seriously annoying bugs when it doesn't work. > > > > Chandler Carruth (4/12/2013): As one of the authors of one of those > compilers, I couldn't agree more. > > ``` > > any interested Googlers can find this here: > >https://yaqs.corp.google.com/eng/q/5768278562045952 > > > > > > Further digging: > > ``` > > To support #pragma once, the compiler tries to identify duplicate > encounters with the same file, but the check gcc actually performs to > establish the identity of the file is weak. Here's someone who made two > copies of the same header with different names, each with a #pragma once, > and it screwed up his build. > > > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52566 > > > > The two headers had the same size, same content, and same timestamps > because he had run "touch *" on them, but they were intended to both be > included. Only one was included, and the other was falsely identified as a > re-inclusion and ignored.One might say he was asking for trouble by running > "touch *", but timestamp collisions are EASY to come by. First of all, > they're only 1sec resolution. You might patch all the relevant files and > they'd have matching timestamps. You might be using a network filesystem > that just doesn't bother with timestamps (common). > > ``` > > > > Now both of these are almost a decade old, so things might have changed > quite a bit since then. > > > > > > Linux kernel threads: > > https://lkml.iu.edu/hypermail/linux/kernel/1401.0/02048.html > > > https://lore.kernel.org/lkml/CAHk-=wi54descexxpmro+q2nag_tup+y5ybhc_9_xglerfp...@mail.gmail.com/ > > > > ``` > > On Sun, Feb 28, 2021 at 11:34 AM Alexey Dobriyan > wrote:>> >> > End result: #pragma is fundamentally less reliable than the> > > traditional #ifdef guard. The #ifdef guard works fine even if you> > > re-read the file for whatever reason, while #pragma relies on some> > kind > of magical behavior.You continue to not even answer this very fundamental > question."#pragma once" doesn't seem to have a _single_ actual real > advantage.Everybody already does the optimization of not even opening - > muchless reading and re-parsing - headers that have the traditional > #ifdefguard.And even if you _don't_ do that optimization, the #ifdef > guardfundmentally semantically guarantyees the right behavior.So the #ifdef > guard is (a) standard (b) simple (c) reliable (d) traditionaland you have > yet to explain a _single_ advantage of "#pragma once".Why add this > incredible churn that has no upside?So no. We're not using #pragma once > unless y9ou can come up with somevery strong argument for itAnd no, having > to come up with a name for the #ifdef guard is not astrong argument. It's > simply not that complicated. Linus > > ``` > > > > > > > > > ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to
[coreboot] Re: [RFC] #pragma once
we have, in the past, used Linux kernel style as our guideline on coreboot style. I'd say, based on Martin's note, that #pragma once is not such a good idea after all. If we decide NOT to use it, however, let's put a note about it in our style guide? This is not the first time this question has come up. On Mon, May 16, 2022 at 12:34 PM Martin Roth wrote: > > After reading what I've included below, I'm going to have to vote to keep the > guards. > Martin > > May 16, 2022, 10:59 by david.hendri...@gmail.com: > > > On Mon, May 16, 2022 at 8:59 AM ron minnich wrote: > > > >> > >> btw, sometimes this has gone the other direction .. > >> https://github.com/lowRISC/opentitan/pull/5693 > >> > > > > It looks like they did that solely to conform to Google's style guide > > which, dogmatic as it may appear, makes sense since OpenTitan is a > > Google-lead project. > > > The question then is 'why does Google require the use of guards?'. Whatever > you think of google, they're not going to mandate something like this without > a good reason. > > I went searching for where this rule came from, and found this: > > ``` > If you trust our in-house C++ compiler gurus, here's the most salient part of > the whole thread linked above.Matt Austern (4/11/2013): If you talk to the > authors of [most C++] compilers, I think you'll find that most of them > consider "#pragma once" (or the equivalent #import) to be a flaky feature -- > something that works almost all of the time and that can cause seriously > annoying bugs when it doesn't work. > > Chandler Carruth (4/12/2013): As one of the authors of one of those > compilers, I couldn't agree more. > ``` > any interested Googlers can find this here: >https://yaqs.corp.google.com/eng/q/5768278562045952 > > > Further digging: > ``` > To support #pragma once, the compiler tries to identify duplicate encounters > with the same file, but the check gcc actually performs to establish the > identity of the file is weak. Here's someone who made two copies of the same > header with different names, each with a #pragma once, and it screwed up his > build. > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52566 > > The two headers had the same size, same content, and same timestamps because > he had run "touch *" on them, but they were intended to both be included. > Only one was included, and the other was falsely identified as a re-inclusion > and ignored.One might say he was asking for trouble by running "touch *", but > timestamp collisions are EASY to come by. First of all, they're only 1sec > resolution. You might patch all the relevant files and they'd have matching > timestamps. You might be using a network filesystem that just doesn't bother > with timestamps (common). > ``` > > Now both of these are almost a decade old, so things might have changed quite > a bit since then. > > > Linux kernel threads: > https://lkml.iu.edu/hypermail/linux/kernel/1401.0/02048.html > https://lore.kernel.org/lkml/CAHk-=wi54descexxpmro+q2nag_tup+y5ybhc_9_xglerfp...@mail.gmail.com/ > > ``` > On Sun, Feb 28, 2021 at 11:34 AM Alexey Dobriyan > wrote:>> >> > End result: #pragma is fundamentally less reliable than the> > > traditional #ifdef guard. The #ifdef guard works fine even if you> > re-read > the file for whatever reason, while #pragma relies on some> > kind of magical > behavior.You continue to not even answer this very fundamental > question."#pragma once" doesn't seem to have a _single_ actual real > advantage.Everybody already does the optimization of not even opening - > muchless reading and re-parsing - headers that have the traditional > #ifdefguard.And even if you _don't_ do that optimization, the #ifdef > guardfundmentally semantically guarantyees the right behavior.So the #ifdef > guard is (a) standard (b) simple (c) reliable (d) traditionaland you have yet > to explain a _single_ advantage of "#pragma once".Why add this incredible > churn that has no upside?So no. We're not using #pragma once unless y9ou can > come up with somevery strong argument for itAnd no, having to come up with a name for the #ifdef guard is not astrong argument. It's simply not that complicated. Linus > ``` > > > > ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: [RFC] #pragma once
After reading what I've included below, I'm going to have to vote to keep the guards. Martin May 16, 2022, 10:59 by david.hendri...@gmail.com: > On Mon, May 16, 2022 at 8:59 AM ron minnich wrote: > >> >> btw, sometimes this has gone the other direction .. >> https://github.com/lowRISC/opentitan/pull/5693 >> > > It looks like they did that solely to conform to Google's style guide > which, dogmatic as it may appear, makes sense since OpenTitan is a > Google-lead project. > The question then is 'why does Google require the use of guards?'. Whatever you think of google, they're not going to mandate something like this without a good reason. I went searching for where this rule came from, and found this: ``` If you trust our in-house C++ compiler gurus, here's the most salient part of the whole thread linked above.Matt Austern (4/11/2013): If you talk to the authors of [most C++] compilers, I think you'll find that most of them consider "#pragma once" (or the equivalent #import) to be a flaky feature -- something that works almost all of the time and that can cause seriously annoying bugs when it doesn't work. Chandler Carruth (4/12/2013): As one of the authors of one of those compilers, I couldn't agree more. ``` any interested Googlers can find this here: https://yaqs.corp.google.com/eng/q/5768278562045952 Further digging: ``` To support #pragma once, the compiler tries to identify duplicate encounters with the same file, but the check gcc actually performs to establish the identity of the file is weak. Here's someone who made two copies of the same header with different names, each with a #pragma once, and it screwed up his build. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52566 The two headers had the same size, same content, and same timestamps because he had run "touch *" on them, but they were intended to both be included. Only one was included, and the other was falsely identified as a re-inclusion and ignored.One might say he was asking for trouble by running "touch *", but timestamp collisions are EASY to come by. First of all, they're only 1sec resolution. You might patch all the relevant files and they'd have matching timestamps. You might be using a network filesystem that just doesn't bother with timestamps (common). ``` Now both of these are almost a decade old, so things might have changed quite a bit since then. Linux kernel threads: https://lkml.iu.edu/hypermail/linux/kernel/1401.0/02048.html https://lore.kernel.org/lkml/CAHk-=wi54descexxpmro+q2nag_tup+y5ybhc_9_xglerfp...@mail.gmail.com/ ``` On Sun, Feb 28, 2021 at 11:34 AM Alexey Dobriyan wrote:>> >> > End result: #pragma is fundamentally less reliable than the> > traditional #ifdef guard. The #ifdef guard works fine even if you> > re-read the file for whatever reason, while #pragma relies on some> > kind of magical behavior.You continue to not even answer this very fundamental question."#pragma once" doesn't seem to have a _single_ actual real advantage.Everybody already does the optimization of not even opening - muchless reading and re-parsing - headers that have the traditional #ifdefguard.And even if you _don't_ do that optimization, the #ifdef guardfundmentally semantically guarantyees the right behavior.So the #ifdef guard is (a) standard (b) simple (c) reliable (d) traditionaland you have yet to explain a _single_ advantage of "#pragma once".Why add this incredible churn that has no upside?So no. We're not using #pragma once unless y9ou can come up with somevery strong argument for itAnd no, having to come up with a name for the #ifdef guard is not astrong argument. It's simply not that complicated. Linus ``` ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: [RFC] #pragma once
Hi Arthur, I'm very much in favor of using #pragma once instead of the include guards in the coreboot tree, since that removes the possibility of some sorts of bugs and also removes 2 lines of boilerplate per header file. Not sure if romcc supported #pragma once and if that was one of the reasons to use the include guards instead, but since romcc was dropped from the main branch a long time ago, that's not a problem. Regards, Felix ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: [RFC] #pragma once
I was sure I'd looked at this at one point and found this from years ago ... "The discussion evolved to a related question, around #pragma once. A few years back, on the Akaros project (kernel written in C, FWIW), a Linux kernel luminary convinced us to get rid of file guards and go to #pragma once. I am not sure it was worth the trouble but we did it. It *can* speed up compile time; cpp doesn't need to process a whole file and then conclude it did not have to process it; it can realize it can skip the open. A significant downside is that it's not in any standard -- just all the compilers out there, it seems, save romcc. I did a simple test: apply #pragma once to coreboot. A coreboot build for watson opens 80K .h files today. #pragma once makes barely any difference; this says we are doing a good job in how we use our .h files." Anyway, all this said, #pragma once seems a good idea. On Mon, May 16, 2022 at 9:59 AM David Hendricks wrote: > > On Mon, May 16, 2022 at 8:59 AM ron minnich wrote: > > > > btw, sometimes this has gone the other direction .. > > https://github.com/lowRISC/opentitan/pull/5693 > > It looks like they did that solely to conform to Google's style guide > which, dogmatic as it may appear, makes sense since OpenTitan is a > Google-lead project. ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: [RFC] #pragma once
On Mon, May 16, 2022 at 8:59 AM ron minnich wrote: > > btw, sometimes this has gone the other direction .. > https://github.com/lowRISC/opentitan/pull/5693 It looks like they did that solely to conform to Google's style guide which, dogmatic as it may appear, makes sense since OpenTitan is a Google-lead project. ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: [RFC] #pragma once
btw, sometimes this has gone the other direction .. https://github.com/lowRISC/opentitan/pull/5693 On Mon, May 16, 2022 at 3:04 AM Angel Pons wrote: > > Hi Arthur, list, > > On Sun, May 15, 2022 at 6:56 PM Arthur Heymans wrote: > > > > Hi > > > > To make sure headers don't create conflicts, guards are added to all of > > them. > > But the guard needs to be correct: e.g. > > https://review.coreboot.org/c/coreboot/+/64360/2 > > Most compilers implement '#pragma once ' as an alternative. > > Should we use this instead across the tree, as it is less error prone and > > less code? > > Given that coreboot is built with a very specific toolchain, it seems > very reasonable. The only thing that worries me are headers used to > build stuff with the system toolchain, e.g. util/ and src/commonlib/ > headers. Still, it's highly unlikely that the system toolchain doesn't > know about #pragma once provided that it is able to build crossgcc. > > > Sidenote: clang warns about wrong header guards. > > https://review.coreboot.org/c/coreboot/+/62173/23 hooks up clang to our CI > > for some platforms ;-). > > And mismatched names in #ifndef and #define is not the only problem. I > recently pondered about the scenario in which a compilation unit > includes two different header files that use the same name in their > guard. Using #pragma once would fundamentally eliminate both problems. > > > Kind regards > > Arthur > > Best regards, > Angel > ___ > coreboot mailing list -- coreboot@coreboot.org > To unsubscribe send an email to coreboot-le...@coreboot.org ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] coreboot 4.17 release planned for May 30th
The next quarterly release, 4.17 is targeted for May 30, 2022. Additionally, at that time, we plan to update the release packages and version numbers any of the coreboot branches which have been modified since they were released. Going forward, this should happen anytime there's a new release. Please test any boards that you have available. Current stats since the 4.16 release, 3 months ago (As of early Monday, May 16): Added 10 mainboards: --- * Clevo L140MU / L141MU / L142MU * Dell Inc. Dell Precision T1650 * Google -> Craask * Google -> Gelarshie * Google -> Mithrax * Google -> Osiris * HP Z220 CMT Workstation * Star Labs Star Labs LabTop Mk IV (i3-10110U and i7-10710U) * Star Labs Star Labs Lite Mk III (N5000) * Star Labs Star Labs Lite Mk IV (N5030) Removed 2 mainboards: --- * Google -> Deltan * Google -> Deltaur repo statistics --- - Total Commits: 998 - Average Commits per day: 12.58 - Total lines added: 34473 - Average lines added per commit: 34.54 - Number of patches adding more than 100 lines: 40 - Average lines added per small commit: 22.35 - Total lines removed: 57663 - Average lines removed per commit: 57.78 - Total difference between added and removed: -23190 - Total authors: 131 - New authors: 17 Martin ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: [RFC] #pragma once
Hi Arthur, list, On Sun, May 15, 2022 at 6:56 PM Arthur Heymans wrote: > > Hi > > To make sure headers don't create conflicts, guards are added to all of them. > But the guard needs to be correct: e.g. > https://review.coreboot.org/c/coreboot/+/64360/2 > Most compilers implement '#pragma once ' as an alternative. > Should we use this instead across the tree, as it is less error prone and > less code? Given that coreboot is built with a very specific toolchain, it seems very reasonable. The only thing that worries me are headers used to build stuff with the system toolchain, e.g. util/ and src/commonlib/ headers. Still, it's highly unlikely that the system toolchain doesn't know about #pragma once provided that it is able to build crossgcc. > Sidenote: clang warns about wrong header guards. > https://review.coreboot.org/c/coreboot/+/62173/23 hooks up clang to our CI > for some platforms ;-). And mismatched names in #ifndef and #define is not the only problem. I recently pondered about the scenario in which a compilation unit includes two different header files that use the same name in their guard. Using #pragma once would fundamentally eliminate both problems. > Kind regards > Arthur Best regards, Angel ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
[coreboot] Re: [RFC] #pragma once
On Sun, 15 May 2022 at 19:56, Arthur Heymans wrote: > Most compilers implement '#pragma once ' as an alternative. I've been using #pragma once since 2019 in fwupd and I've never once had a problem with it -- and we compile with a lot of weird compilers and for a lot of strange targets. It reduced the amount of boilerplate by a huge amount, and removed one new-contributor "gotcha" that was lurking for every person that added a new header. Richard. ___ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
