Re: [PATCH 00/11] pragma once: treewide conversion

Re: [PATCH 00/11] pragma once: treewide conversion

[Linus Torvalds]

On Sun, Feb 28, 2021 at 8:57 AM Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> This is bulk deletion of preprocessor include guards and conversion
> to #pragma once directive.

So as mentioned earlier, I'm not 100% convinced about the advantage of
#pragma once.

But I decided to actually test it, and it turns out that it causes
problems for at least sparse.

Sparse *does* support pragma once, but it does it purely based on
pathname equality. So a simple test-program like this:

 File 'pragma.h':

    #pragma once
    #include "header.h"

works fine. But this doesn't work at all:

    #pragma once
    #include "./header.h"

because it causes the filename to be different every time, and you
eventually end up with trying to open   "././....../pragma.h" and it
causes ENAMETOOLONG.

So at least sparse isn't ready for this.

I guess sparse could always simplify the name, but that's non-trivial.

And honestly, using st_dev/st_ino is problematic too, since

 (a) they can easily be re-used for generated files

 (b) you'd have to actually open/fstat the filename to use it, which
obviates one of the optimizations

Trying the same on gcc, you don't get that endless "add "./" behavior"
that sparse did, but a quick test shows that it actually opens the
file and reads it three times: once for "pramga.h", once for
"./pragma.h" and a third time for "pragma.h". It only seems to
_expand_ it once, though.

I have no idea what gcc does. Maybe it does some "different name, so
let's open and read it, and then does st_dev/st_ino again". But if so,
why the _third_ time? Is it some guard against "st_ino might have been
re-used, so I'll open the original name and re-verify"?

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.

I'm adding Luc in case he has any ideas of what the magical behavior might be.

Honestly, I think #pragma once is complete garbage. It's really is
fundamenetally more complicated than the #ifdef guard, and it has
absolutely zero upsides.

I'm not taking this pramga series unless somebody can explain why it's
a good thing. Because all I see are downsides.

               Linus

Re: [PATCH 00/11] pragma once: treewide conversion

[Alexey Dobriyan]

On Sun, Feb 28, 2021 at 09:46:17AM -0800, Linus Torvalds wrote:
> On Sun, Feb 28, 2021 at 8:57 AM Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >
> > This is bulk deletion of preprocessor include guards and conversion
> > to #pragma once directive.
> 
> So as mentioned earlier, I'm not 100% convinced about the advantage of
> #pragma once.
> 
> But I decided to actually test it, and it turns out that it causes
> problems for at least sparse.

Oh no.

> Sparse *does* support pragma once, but it does it purely based on
> pathname equality.

Doing what gcc or clang does seems like a smart thing to do.

> So a simple test-program like this:
> 
>  File 'pragma.h':
> 
>     #pragma once
>     #include "header.h"
> 
> works fine. But this doesn't work at all:
> 
>     #pragma once
>     #include "./header.h"
> 
> because it causes the filename to be different every time, and you
> eventually end up with trying to open   "././....../pragma.h" and it
> causes ENAMETOOLONG.
> 
> So at least sparse isn't ready for this.
> 
> I guess sparse could always simplify the name, but that's non-trivial.
> 
> And honestly, using st_dev/st_ino is problematic too, since
> 
>  (a) they can easily be re-used for generated files
> 
>  (b) you'd have to actually open/fstat the filename to use it, which
> obviates one of the optimizations

fstat is more or less necessary anyway to allocate just enough memory
for 1 read. fstat is not a problem, read is (and subsequent parsing).

> Trying the same on gcc, you don't get that endless "add "./" behavior"
> that sparse did, but a quick test shows that it actually opens the
> file and reads it three times: once for "pramga.h", once for
> "./pragma.h" and a third time for "pragma.h". It only seems to
> _expand_ it once, though.
> 
> I have no idea what gcc does. Maybe it does some "different name, so
> let's open and read it, and then does st_dev/st_ino again". But if so,
> why the _third_ time? Is it some guard against "st_ino might have been
> re-used, so I'll open the original name and re-verify"?
> 
> 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.
> 
> I'm adding Luc in case he has any ideas of what the magical behavior might be.

gcc does

	open "/" + "whatever between quotes"
	fstat

so that "1.h" and "./1.h" differ

	https://github.com/gcc-mirror/gcc/blob/master/libcpp/files.c#L377

clang does better:

	"./" + "whatever between quotes"
	open
	fstat
	normalise pathname via readlink /proc/*/fd

I think it is quite hard to break something with double inclusion
without trying to actually break stuff. Macros has to be token
for token identical or compiler warn. Types definition too.
Function prototypes and so on.

This is how I found half of the exception list.

The "no leading ./ in includes is trivially enforced with checkpatch.pl
or even grep! And it will optimise the build now that gcc behaviour has
been uncovered.

Include guards aren't without problems.

We have at least 1 identical include guard in the tree for two
completely unrelated headers (allmodconfig of some fringe arch found it)
Nobody complains because only defconfigs are run apparently.

Developer can typo it disabling double inclusion.

	#ifndef FOO_H
	#define FOO_h
	#endif

I've seen a reference to a static checker checking for such stuff
so this problem exists.

Invisibly broken inlcude guards (see qla2xxx patch in the series).

Re: [PATCH 00/11] pragma once: treewide conversion

[Linus Torvalds]

On Sun, Feb 28, 2021 at 11:34 AM Alexey Dobriyan <adobriyan@gmail.com> 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 - much
less reading and re-parsing - headers that have the traditional #ifdef
guard.

And even if you _don't_ do that optimization, the #ifdef guard
fundmentally semantically guarantyees the right behavior.

So the #ifdef guard is
 (a) standard
 (b) simple
 (c) reliable
 (d) traditional

and 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 some
very strong argument for it

And no, having to come up with a name for the #ifdef guard is not a
strong argument. It's simply not that complicated.

               Linus

Re: [PATCH 00/11] pragma once: treewide conversion

[Linus Torvalds]

On Wed, Mar 3, 2021 at 11:46 AM Tom Tromey <tom@tromey.com> wrote:
>
> It's also worth noting that in GCC it is slower than include guards.
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58770
>
> It's just a bug, probably easy to fix.  On the other hand, nobody has
> ever bothered to do so.

That bugzilla is actually worth reading, if only to explain how the
include guard is more robust technology compared to #pragma once.

The traditional include guarding with #ifndef/#define/#endif around
the contents has the advantage that a compiler _can_ generate the
obvious trivial optimizations of just memoizing that "oh, I've seen
this filename already, and it had that include guard pattern, so I
don't need to include it again".

But, I hear you say "that's exactly what '#pragma once' does too!".

No, it's not. There's actually two huge and very fundamental
differences between '#pragma once' and the traditional include guard
optimization:

 (a) the traditional include guard optimization HAS NO HIDDEN SEMANTIC
MEANING. It's a pure optimization that doesn't actually change
anything else. If you don't do the optimization, absolutely nothing
changes.

 (b) the traditional include guard model allows for overriding and is
simply more flexible

And the GCC bugzilla talks about some of the issues with (a), and I
already mentioned one similar issue with (a) wrt sparse: exactly what
is it that "#pragma once" really protects?

Is it the filename? Is it the _canonical_ filename? What about
symbolic links or hardlinks? Is it the inode number? What about
filesystems that don't really have those concepts?

The above questions aren't some made-up example. They are literally
FUNDAMENTAL DESIGN MISTAKES in "#pragma once".

In contrast, include guards just work. You give the guard an explicit
name, and that solves all the problems above, and allows for extra
flexibility (ie the (b) issue: you can override things and include
things twice if you know you're playing games, but you can also use
the guard name to see "have I already included this file" for when you
have possible nasty circular include file issues etc).

So the traditional include guard model is simply technically the superior model.

This is why I'm NAK'ing "#pragma once". It was never a good idea, and
the alleged advantage ("faster builds by avoiding double includes")
was always pure garbage because preprocessors could do the same
optimization using the traditional include guards. In fact, because
the traditional include guards have well-defined meaning and doesn't
have any of the questions about what makes a file unique, and a missed
optimization doesn't cause any semantic differences, a compiler has a
much _easier_ time with that optimization than with the ostensibly
simpler "#pragma once".

Most #pragma things are not wonderful. But '#pragma once' is actively bad.

             Linus

RE: [PATCH 00/11] pragma once: treewide conversion

[David Laight]

>  (a) the traditional include guard optimization HAS NO HIDDEN SEMANTIC
> MEANING. It's a pure optimization that doesn't actually change
> anything else. If you don't do the optimization, absolutely nothing
> changes.

And if the parser is well written the optimisation is probably
irrelevant compared to the compile time.

OTOH that probably requires using mmap(), memchr('\n') to look
for line starts, a fast search for '[ ]*#' followed by else/endif
and a final horrid backwards check for a continuation line.

That optimisation will generally speed up header file processing.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH 00/11] pragma once: treewide conversion

[Linus Torvalds]

On Thu, Mar 4, 2021 at 5:55 AM David Laight <David.Laight@aculab.com> wrote:
>
> >  (a) the traditional include guard optimization HAS NO HIDDEN SEMANTIC
> > MEANING. It's a pure optimization that doesn't actually change
> > anything else. If you don't do the optimization, absolutely nothing
> > changes.
>
> And if the parser is well written the optimisation is probably
> irrelevant compared to the compile time.

That's actually surprisingly not even remotely true.

People always think that the optimization phases of a compiler are the
expensive ones. And yes, there are certain optimizations that can be
*really* expensive, and people just don't even do them because they
are _so_ expensive and are exponential in time.

And yes, there can be some patterns that expose bad scaling in some
compiler algorithms, and histyorically that has often been seen when
people use generators to automatically generate huge functions (or
huge initializers), and then the compiler has some O(n**3) thing in it
that is entirely unnoticeable for normal code written by a human, but
means that a million-entry initializer takes five hours to compile.

But in reality, on very real normal code, the *front end* of the
compiler is often the most costly thing by far. Very much just the
lexer and the simple parser. Things that are "simple" and people think
are fast.

But they are are often the slowest part in C style languages, because
you have to lex and parse _everything_. You include maybe a dozen
header files, maybe more. Those in turn often include another dozen
support header files. You easily end up tokenizing and parsing
hundreds of header files for even the simplest programs - and 99,.9%
of that isn't ever *used*, and never actually generates any code that
needs to be optimized. Think of all the complex macros and functions
and type definitions you get when you include something basic like
<stdio.h>. Trust me, it's a _lot_ of small details that get tokenixed,
parsed and memoized.

And then all you do is a 'printf("Hello world");' and the amount of
actual code generation and optimization by the back-end is basically
zero.

And C++ is about an order of magnitude worse, because you really take
this whole approach and turn it up to 11 with templates, namespaces,
STL, and a lot of all that infrastructure that is there for all the
possible cases, but that most files that include it only use a small
tiny portion of.

So in C-style (and particularly C++) languages, reading header files,
tokenizing them and parsing them can _easily_ be the bulk of your
compile time. Absolutely every single serious C/C++ compiler
integrates tbe preprocessor _into_ the compiler, because the
traditional model of having a separate "cpp" phase actually made the
tokenization problem happen _twice_: once by the pre-processor, and
then a second time by the compiler. Integrating the two phases, so
that you can use just one tokenizer, and one single set of
identifiers, actually speeds up compile speed _enormously_.

Yes, yes, tokenization and parsing really is the simple part of a
compiler. Good register allocation is _hard_. SSA makes a lot of the
basic optimizations actually fairly straightforward and simple, but
more complex transformations (loop invariant stuff, vectorization,
various things like that) are really much much much more complex than
the simple front-end.

But that simple front end is the thing that touches absolutely
_everything_, whether it actually generates code or not.

And yes, this is mainly a C-style language problem. If you have
hardcoded modules approach and not the "include files that describe
all the different interfaces", you don't end up in that situation
where you spend a lot of time parsign the possible interfaces over and
over again.

And C++ really is hugely worse, and takes this issue to another level.
You can have simple single C++ files that end up basically having to
parse hundreds of thousands of lines of fairly complex code, because
they use several complex libraries, and that's how the library header
files are set up,m with multiple levels (ie the GUI header files
depend on, and include the lower-level object header files, which in
turn depend on "simple" core libraries like STL and Boost.

This is why pretty much every compiler out there does that include
file header guard optimization. Because avoiding having to read and
parse a file multiple times really does show up as a big big win. Even
when it's all hidden behind the #ifndef/#define/#endif guarding logic,
the cost of reading the file and lexing and parsing it enough to _see_
those guards is not cheap. Doing it once is required and important,
but then you memoize the fact that "oh, all the stuff I needed to
parse was behind this macro test, so next time I see this file, if
that macro is set, I can just ignore it".

So it is actually a very important optimization. It's just that the
optimization is better done with that explicit guard memoization than
it is with '#pragma once'

                       Linus

RE: [PATCH 00/11] pragma once: treewide conversion

[David Laight]

From: Linus Torvalds
> Sent: 04 March 2021 20:16
> 
> On Thu, Mar 4, 2021 at 5:55 AM David Laight <David.Laight@aculab.com> wrote:
> >
> > >  (a) the traditional include guard optimization HAS NO HIDDEN SEMANTIC
> > > MEANING. It's a pure optimization that doesn't actually change
> > > anything else. If you don't do the optimization, absolutely nothing
> > > changes.
> >
> > And if the parser is well written the optimisation is probably
> > irrelevant compared to the compile time.
> 
> That's actually surprisingly not even remotely true.

The point is that you can skip the unwanted parts of
#if without having to parse the file at all.
You just need to detect the line breaks.

So yes, you need to read the file and look at every byte.
But you don't need to even start tokenising it.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH 00/11] pragma once: treewide conversion

[Linus Torvalds]

On Fri, Mar 5, 2021 at 1:19 AM David Laight <David.Laight@aculab.com> wrote:
>
> The point is that you can skip the unwanted parts of
> #if without having to parse the file at all.
> You just need to detect the line breaks.

That's not actually true AT ALL.

You still need to at the very least parse the preprocessor tokens,
looking for things like #if, #else, and #endif. Because those can -
and do - nest inside the whole thing, so you're not even looking for
the final #endif, you have to be aware that there might be new #if
statements that means that now you now have to increment the nesting
count for the endif.

And to do even just *THAT*, you need to do all the comment parsing,
and all the string parsing, because a #endif means something entirely
different if there was a "/*"  or a string on a previous line that
hasn't been terminated yet (yes, unterminated strings are bad
practice, but ..).

And regardless of even _those_ issues, you still should do all the
other syntactic tokenization stuff (ie all the quoting, the the
character handling: 'a' is a valid C token, but if you see the string
"it's" outside of a comment, that's a syntax error even if it's inside
a disabled region. IOW, this is an incorrect file:

   #if 0
   it's a bug to do this, and the compiler should scream
   #endif

because it's simply not a valid token sequence. The fact that it's
inside a "#if 0" region doesn't change that fact at all.  So you did
need to do all the tokenization logic.

The same goes for all the wide string stuff, the tri-graph horrors, etc etc.

End result: you need to still do basically all of the basic lexing,
and while you can then usually quickly throw the result mostly away
(and you could possibly use a simplified lexer _because_ you throw it
away), you actually didn't really win much. Doing a specialized lexer
just for the disabled regions is probably simply a bad idea: the fact
that you need to still do all the #if nesting etc checking means that
you still do need to do a modicum of tokenization etc.

Really: the whole "trivial" front-end parsing phase of C - and
particularly C++ - is a huge huge deal. It's going to show in the
profiles of the compiler quite decisively, unless you have a compiler
that then spends absolutely insane time on optimization and tries to
do things that basically no sane compiler does (because developers
wouldn't put up with the time sink).

So yes, I've even used things like super-optimizers that chew on small
pieces of code for _days_ because they have insane exponential costs
etc. I've never done it seriously, because it really isn't realistic,
but it can be a fun exercise to try.

Outside of those kinds of super-optimizers, lexing and parsing is a
big big deal.

(And again - this is very much language-specific.  The C/C++ model of
header files is very very flexible, and has a lot of conveniences, but
it's also a big part of why the front end is such a big deal. Other
language models have other trade-offs).

             Linus

Re: [PATCH 00/11] pragma once: treewide conversion

[Miguel Ojeda]

On Fri, Mar 5, 2021 at 10:25 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> And regardless of even _those_ issues, you still should do all the
> other syntactic tokenization stuff (ie all the quoting, the the
> character handling: 'a' is a valid C token, but if you see the string
> "it's" outside of a comment, that's a syntax error even if it's inside
> a disabled region. IOW, this is an incorrect file:
>
>    #if 0
>    it's a bug to do this, and the compiler should scream
>    #endif
>
> because it's simply not a valid token sequence. The fact that it's
> inside a "#if 0" region doesn't change that fact at all.  So you did
> need to do all the tokenization logic.

Compilers don't scream that much, only GCC seems to give a warning. I
assume it is because it is just undefined rather than a required
error/diagnostic, i.e. the "If a ’ or a " character matches the last
category, the behavior is undefined." in 6.4.

Concerning #pragma once: I actually would like to have a standard
#once directive if what is a "seen file" could be defined a bit more
precisely. Even if just says it creates a guard with something similar
to the result of `__FILE__` would be good enough for many projects out
there, and one can still use guards when flexibility is needed and/or
corner cases are expected (which, if detected, the compiler could also
warn about).

Cheers,
Miguel

Re: [PATCH 00/11] pragma once: treewide conversion

[Linus Torvalds]

On Sat, Mar 6, 2021 at 5:07 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Concerning #pragma once: I actually would like to have a standard
> #once directive if what is a "seen file" could be defined a bit more
> precisely.

I think it would be ok if you had something like

   #pragma once IDTOKEN

which would basically act *exactly* as a "#ifndef IDTOKEN/#define
IDTOKEN" with that final #endif at the end of the file.

But even then it should have the rule that it must be the very first
thing in the file (ignoring whitespace and pure comments).

Then it would literally be just a cleaner and simpler version of the
guarding logic, with none of the semantic confusion that #pragma once
now has.

Because I cannot see any other way to define what "seen file" really means.

There's no sane "implied IDTOKEN" that I can see. It can't really be
the pathname, because pathnames are actually really hard to turn into
canonical form in user space thanks to things like symlinks. It can't
be st_ino/st_dev stat information, because the C preprocessor isn't a
"POSIX only" thing.

It could be a "Ok, #pragma once only works if you don't have multiple
ways to reach the same file, and you never have ambiguous include
paths". But that's actually not all that unusual in real projects: you
often have fairly complex include paths, you have C files that include
the header in the same directory with just "filename.h", but it
_could_ get included indirectly from _another_ header that gave a
pathname relative to the project root, etc etc.

This is not unrelated to another complete morass of horrible problems:
precompiled header files. That feature had exactly the same reason for
it as "#pragma once" - slow build speeds due to the cost of parsing
complex header file hierarchies.

And I guarantee that multiple compiler teams spent hundreds of
person-years of effort on trying to make it work. And several
compilers do support the notion. And they ALL have big warnings about
when you should enable it, and when you shouldn't, and what things
don't work if you use them, and about how it can cause really really
subtle problems.

Because it turns out that precompiled header files are a major pain
and a major mistake. Some projects still use them, because they can be
such a huge timesaver (again: particularly C++ projects that just have
a "include everything" approach to headers because trying to separate
things out is too hard). But they all come with huge warnings about
how they break the actual real semantics of a compiler, and if you do
_anything_ to change the build ("hey, I'd like to use a different
compiler option"), things may or may not work.

           Linus

Re: [PATCH 00/11] pragma once: treewide conversion

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

Hi!

> > >  (a) the traditional include guard optimization HAS NO HIDDEN SEMANTIC
> > > MEANING. It's a pure optimization that doesn't actually change
> > > anything else. If you don't do the optimization, absolutely nothing
> > > changes.
> >
> > And if the parser is well written the optimisation is probably
> > irrelevant compared to the compile time.
> 
> That's actually surprisingly not even remotely true.
> 
> People always think that the optimization phases of a compiler are the
> expensive ones. And yes, there are certain optimizations that can be
> *really* expensive, and people just don't even do them because they
> are _so_ expensive and are exponential in time.

Well, linux kernel can be compiled in two _seconds_ if you take
compiler optimized for fast parsing... and quick code generation.

See "SUSE Labs Conference 2018 - Compiling the Linux kernel in a
second (give or take)" on youtube.

So yes, gcc's frontend may be slow, but that does not mean job can not
be done quickly by suitable compiler.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 00/11] pragma once: treewide conversion

[Luc Van Oostenryck]

On Sun, Feb 28, 2021 at 10:34:46PM +0300, Alexey Dobriyan wrote:
> 
> gcc does
> 
> 	open "/" + "whatever between quotes"
> 	fstat
> 
> so that "1.h" and "./1.h" differ

When I try the following with GCC 10.2:
	$ cat header.h
	#pragma once
	#include "./header.h"
	.. plenty of stuff ..

	$ strace -f gcc -E header.h

I see that the file is opened 4 times (3 times with the name "header.h"
and once with "./header.h"). Each of these open is followed by a fstat().
More annoyingly, the file is also read 4 times (3 times entirely and once
only the fist 4096 bytes).

When the equivalent is done with an include guard instead, the file is
only read twice (once with each names) and read twice (entirely).

-- Luc