David, apropos your comment that we can assume clang will catch any real problems; your comment reminds me of a lesson we learned on Harvey-OS.
What we learned on Harvey-OS, when we were compiling a working kernel across 3 versions of gcc, and 3 versions of clang, was that at one time or another, one of these 6 compilers gave us a warning on some critical code issue in Harvey-0S, that the other 5 did not. Older compilers warned of something a newer one did not; gcc and clang each had warnings the other missed. This was surprising and a useful lesson to us. It's also why I'll never be able to accept the idea that coreboot should be written in a gcc-specific dialect. But that's another argument for another day. On Mon, May 16, 2022 at 9:53 PM David Hendricks <[email protected]> wrote: > > 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 <[email protected]> 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 <[email protected]> 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 [email protected]: >> > >> > > On Mon, May 16, 2022 at 8:59 AM ron minnich <[email protected]> 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 <[email protected]> >> > 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 itA nd 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 -- [email protected] To unsubscribe send an email to [email protected]
