[PATCH] proc: use #pragma once

[PATCH] proc: use #pragma once

[Alexey Dobriyan]

Bring /proc into 21st century.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 fs/proc/fd.h            |    6 +-----
 fs/proc/internal.h      |    1 +
 include/linux/proc_fs.h |    6 +-----
 3 files changed, 3 insertions(+), 10 deletions(-)

--- a/fs/proc/fd.h
+++ b/fs/proc/fd.h
@@ -1,7 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __PROCFS_FD_H__
-#define __PROCFS_FD_H__
-
+#pragma once
 #include <linux/fs.h>
 
 extern const struct file_operations proc_fd_operations;
@@ -16,5 +14,3 @@ static inline unsigned int proc_fd(struct inode *inode)
 {
 	return PROC_I(inode)->fd;
 }
-
-#endif /* __PROCFS_FD_H__ */
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -1,3 +1,4 @@
+#pragma once
 /* Internal procfs definitions
  *
  * Copyright (C) 2004 Red Hat, Inc. All Rights Reserved.
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -1,10 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+#pragma once
 /*
  * The proc filesystem constants/structures
  */
-#ifndef _LINUX_PROC_FS_H
-#define _LINUX_PROC_FS_H
-
 #include <linux/types.h>
 #include <linux/fs.h>
 
@@ -82,5 +80,3 @@ static inline struct proc_dir_entry *proc_net_mkdir(
 struct ns_common;
 int open_related_ns(struct ns_common *ns,
 		   struct ns_common *(*get_ns)(struct ns_common *ns));
-
-#endif /* _LINUX_PROC_FS_H */

Re: [PATCH] proc: use #pragma once

[Christoph Hellwig]

On Tue, Apr 24, 2018 at 12:35:34AM +0300, Alexey Dobriyan wrote:
> Bring /proc into 21st century.

Please explain what this actually buys us except for being pointlessly
different from the rest of the kernel?

Re: [PATCH] proc: use #pragma once

[Alexey Dobriyan]

On Tue, Apr 24, 2018 at 06:54:09AM -0700, Christoph Hellwig wrote:
> On Tue, Apr 24, 2018 at 12:35:34AM +0300, Alexey Dobriyan wrote:
> > Bring /proc into 21st century.
> 
> Please explain what this actually buys us except for being pointlessly
> different from the rest of the kernel?

Negative LOC diff, less junk in preprocessor hashtables.

Re: [PATCH] proc: use #pragma once

[David Sterba]

On Wed, Apr 25, 2018 at 11:55:31PM +0300, Alexey Dobriyan wrote:
> On Tue, Apr 24, 2018 at 06:54:09AM -0700, Christoph Hellwig wrote:
> > On Tue, Apr 24, 2018 at 12:35:34AM +0300, Alexey Dobriyan wrote:
> > > Bring /proc into 21st century.
> > 
> > Please explain what this actually buys us except for being pointlessly
> > different from the rest of the kernel?
> 
> Negative LOC diff, less junk in preprocessor hashtables.

There are about 20k header files, none of them has #pragma once.
Updating that will bring many unnesessry git commits.

I doubt that one more define in preprocessor tables is a problem we
should fix. The LOC argument also does not sound very convincing. The
include protection is at the top of the file, not mixed among other
code. Replacing 2-3 lines with one will be barely noticeable.

Re: [PATCH] proc: use #pragma once

[Christoph Hellwig]

On Thu, Apr 26, 2018 at 12:26:29PM +0200, David Sterba wrote:
> There are about 20k header files, none of them has #pragma once.
> Updating that will bring many unnesessry git commits.
> 
> I doubt that one more define in preprocessor tables is a problem we
> should fix. The LOC argument also does not sound very convincing. The
> include protection is at the top of the file, not mixed among other
> code. Replacing 2-3 lines with one will be barely noticeable.

Let's put it another way:  does this make any noticable difference
for compile times?

Re: [PATCH] proc: use #pragma once

[Alexey Dobriyan]

On Thu, Apr 26, 2018 at 03:33:04AM -0700, Christoph Hellwig wrote:
> On Thu, Apr 26, 2018 at 12:26:29PM +0200, David Sterba wrote:
> > There are about 20k header files, none of them has #pragma once.
> > Updating that will bring many unnesessry git commits.
> > 
> > I doubt that one more define in preprocessor tables is a problem we
> > should fix. The LOC argument also does not sound very convincing. The
> > include protection is at the top of the file, not mixed among other
> > code. Replacing 2-3 lines with one will be barely noticeable.
> 
> Let's put it another way:  does this make any noticable difference
> for compile times?

Should be the same as include guard pattern is recognized and becomes
#pragma once internally.

Re: [PATCH] proc: use #pragma once

[Alexey Dobriyan]

On Thu, Apr 26, 2018 at 12:26:29PM +0200, David Sterba wrote:
> On Wed, Apr 25, 2018 at 11:55:31PM +0300, Alexey Dobriyan wrote:
> > On Tue, Apr 24, 2018 at 06:54:09AM -0700, Christoph Hellwig wrote:
> > > On Tue, Apr 24, 2018 at 12:35:34AM +0300, Alexey Dobriyan wrote:
> > > > Bring /proc into 21st century.
> > > 
> > > Please explain what this actually buys us except for being pointlessly
> > > different from the rest of the kernel?
> > 
> > Negative LOC diff, less junk in preprocessor hashtables.
> 
> There are about 20k header files, none of them has #pragma once.
> Updating that will bring many unnesessry git commits.

If you don't obey sometimes silly rule re splitting patches,
it can be done in much less.

> I doubt that one more define in preprocessor tables is a problem we
> should fix.

Those eat cycles and memory one define at a time.

> The LOC argument also does not sound very convincing.

When was the last time you did -80 kLOC patch for free?

> The include protection is at the top of the file, not mixed among other
> code. Replacing 2-3 lines with one will be barely noticeable.

Re: [PATCH] proc: use #pragma once

[Andrew Morton]

On Thu, 26 Apr 2018 22:24:44 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:

> > The LOC argument also does not sound very convincing.
> 
> When was the last time you did -80 kLOC patch for free?

That would be the way to do it - sell the idea to Linus, send him a
script to do it then stand back.  The piecemeal approach is ongoing
pain.

Re: [PATCH] proc: use #pragma once

[Rasmus Villemoes]

On 2018-05-02 00:13, Andrew Morton wrote:
> On Thu, 26 Apr 2018 22:24:44 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
> 
>>> The LOC argument also does not sound very convincing.
>>
>> When was the last time you did -80 kLOC patch for free?
> 
> That would be the way to do it - sell the idea to Linus, send him a
> script to do it then stand back.  The piecemeal approach is ongoing
> pain.
> 

FWIW, it's not just removing some identifiers from cpp's hash tables, it
also reduces I/O: Due to our header mess, we have some cyclic includes,
e.g mm.h -> memremap.h -> mm.h. While parsing mm.h, cpp sees the #define
_LINUX_MM_H, then goes parsing memremap.h, but since it hasn't reached
the end of mm.h yet (seeing that there's nothing but comments outside
the #ifndef/#endif pair), it hasn't had a chance to set the internal
flag for mm.h, so it goes slurping in mm.h again. Obviously, the
definedness of _LINUX_MM_H at that point means it "only" has to parse
those 87K for comments and matching up #ifs, #ifdefs,#endifs etc. With
#pragma once, the flag gets set for mm.h immediately, so the #include
from memremap.h is entirely ignored. This can easily be verified with
strace. And mm.h is not the only header getting read twice.

I had some "extract the include guard" line noise lying around, so I
hacked up the below if someone wants to play some more with this. A few
not-very-careful kbuild timings didn't show anything significant, but
both the before and after times were way too noisy, and I only patched
include/linux/*.h.

Anyway, the first order of business is to figure out which ones to leave
alone. We have a bunch of #ifndef THAT_ONE #error "don't include
$this_one directly". The brute-force way is to simply record all macros
which are checked for definedness at least twice.

git grep -h -E '^\s*#\s*if(.*defined\s*\(|n?def)\s*[A-Za-z0-9_]+' | grep
-o -E '[A-Za-z_][A-Za-z_0-9]*' | sort | uniq --repeated > multest.txt

But there's also stuff like arch/x86/boot/compressed/kaslr.c that plays
games with pre-defining _EXPORT_H to avoid parsing export.h when it
inevitably gets included. Oh well, just add the list of macros that have
at least two definitions.

git grep -h -E '^\s*#\s*define\s+[A-Za-z0-9_]+' | grep -o -E
'^\s*#\s*define\s+[A-Za-z0-9_]+' | grep -oE '[A-Za-z0-9_]+' | sort |
uniq --repeated > muldef.txt

With those, one can just do

cat muldef.txt multest.txt | scripts/replace_ig.pl ...

This ends up detecting a lot of copy-pasting (e.g.
__LINUX_MFD_MAX8998_H), as well as lots of headers that for no obvious
reason do not have an include guard. Oh, and once.h has a redundant \.

Rasmus

wear sunglasses...

=== scripts/replace_ig.pl ===

#!/usr/bin/perl

use strict;
use warnings;
use File::Slurp;

my %preserve;

sub strip_comments {
    my $txt = shift;

    # Line continuations are handled before comment stripping, so
    # <slash> <backslash> <newline> <star> actually starts a comment,
    # and a // comment can swallow the following line. Let's just
    # assume nobody has modified the #if control flow using such dirty
    # tricks when we do a more naive line-by-line parsing below to
    # actually remove the include guard deffery.
    $txt =~ s/\\\n//g;

    # http://stackoverflow.com/a/911583/722859
    $txt =~ s{
		 /\*         ##  Start of /* ... */ comment
		 [^*]*\*+    ##  Non-* followed by 1-or-more *'s
		 (?:
		     [^/*][^*]*\*+
		 )*          ##  0-or-more things which don't start with /
		 ##    but do end with '*'
		 /           ##  End of /* ... */ comment

	     |
		 //     ## Start of // comment
		 [^\n]* ## Anything which is not a newline
		 (?=\n) ## End of // comment; use look-ahead to avoid consuming the
newline

	     |         ##     OR  various things which aren't comments:

		 (
		     "           ##  Start of " ... " string
		     (?:
			 \\.           ##  Escaped char
		     |               ##    OR
			 [^"\\]        ##  Non "\
		     )*
		     "           ##  End of " ... " string

		 |         ##     OR

		     '           ##  Start of ' ... ' string
		     (
			 \\.           ##  Escaped char
		     |               ##    OR
			 [^'\\]        ##  Non '\
		     )*
		     '           ##  End of ' ... ' string

		 |         ##     OR

		     .           ##  Anything other char
		     [^/"'\\]*   ##  Chars which doesn't start a comment, string or escape
		 )
	 }{defined $1 ? $1 : " "}gxse;

    return $txt;
}

sub include_guard {
    my $txt = shift;
    my @lines = (split /^/, $txt);
    my $i = 0;
    my $level = 1;
    my $name;

   # The first non-empty line must be an #ifndef or an #if !defined().
    ++$i while ($i < @lines && $lines[$i] =~ m/^\s*$/);
    goto not_found if ($i == @lines);
    goto not_found
	if (!($lines[$i] =~
m/^\s*#\s*ifndef\s+(?<name>[A-Za-z_][A-Za-z_0-9]*)\s*$/) &&
	    !($lines[$i] =~
m/^\s*#\s*if\s+!\s*defined\s*\(\s*(?<name>[A-Za-z_][A-Za-z_0-9]*)\s*\)\s*$/));
    $name = $+{name};

    # The next non-empty line must be a #define of that macro.
    1 while (++$i < @lines && $lines[$i] =~ m/^\s*$/);
    goto not_found if ($i == @lines);
    goto not_found if !($lines[$i] =~ m/^\s*#\s*define\s+\b$name\b/);

    # Now track #ifs and #endifs. #elifs and #elses don't change the level.
    while (++$i < @lines && $level > 0) {
	if ($lines[$i] =~ m/^\s*#\s*(?:if|ifdef|ifndef)\b/) {
	    $level++;
	} elsif ($lines[$i] =~ m/^\s*#\s*endif\b/) {
	    $level--;
	}
    }
    goto not_found if ($level > 0); # issue a warning?
    # Check that the rest of the file consists of empty lines.
    ++$i while ($i < @lines && $lines[$i] =~ m/^\s*$/);
    goto not_found if ($i < @lines);
    return $name;

 not_found:
    return undef;
}

sub do_file {
    my $fn = shift;
    my $src = read_file($fn);
    my $ig = include_guard(strip_comments($src));
    if (not defined $ig) {
	printf STDERR "%s: no include guard\n", $fn;
	return;
    }
    if (exists $preserve{$ig}) {
	printf STDERR "%s: include guard %s exempted\n", $fn, $ig;
	return;
    }

    # OK, the entire text should match this horrible regexp.
    if ($src =~ m{
  (.*?) # arbitrary stuff before #ifndef
  (^\s*\#\s*if(?:\s*!\s*defined\s*\(\s*$ig\s*\)|ndef\s*$ig) .*? \n #
  (?:^\s*\n)*
   ^\s*\#\s*define\s*$ig .*? \n) # 2/3 of include guard
  (.*(?=^\s*\#\s*endif)) # body of file
  (^\s*\#\s*endif .*? \n) # last 1/3
  (.*) # rest of file (trailing comments)
	}smx) {
	my $pre = $1;
	my $define = $2;
	my $body = $3;
	my $endif = $4;
	my $post = $5;
	$body =~ s/\n[ \t]*\n$/\n/g;
	$src = $pre . "#pragma once\n";
	$src .= $body . $post;
    } else {
	printf STDERR "%s: has include guard %s, but I failed to replace it
with #pragma once\n",
	$fn, $ig;
	return;
    }
    write_file($fn, $src);
}

while (<STDIN>) {
    chomp;
    $preserve{$_} = 1;
}

for (@ARGV) {
    do_file($_);
}

Re: [PATCH] proc: use #pragma once

[Al Viro]

On Fri, May 04, 2018 at 12:14:57AM +0200, Rasmus Villemoes wrote:

> FWIW, it's not just removing some identifiers from cpp's hash tables, it
> also reduces I/O: Due to our header mess, we have some cyclic includes,
> e.g mm.h -> memremap.h -> mm.h. While parsing mm.h, cpp sees the #define
> _LINUX_MM_H, then goes parsing memremap.h, but since it hasn't reached
> the end of mm.h yet (seeing that there's nothing but comments outside
> the #ifndef/#endif pair), it hasn't had a chance to set the internal
> flag for mm.h, so it goes slurping in mm.h again. Obviously, the
> definedness of _LINUX_MM_H at that point means it "only" has to parse
> those 87K for comments and matching up #ifs, #ifdefs,#endifs etc. With
> #pragma once, the flag gets set for mm.h immediately, so the #include
> from memremap.h is entirely ignored. This can easily be verified with
> strace. And mm.h is not the only header getting read twice.

Which gcc version it is?  Does it *really* read anything twice?  After all,
you are going to read (and tokenize) the rest of mm.h anyway, so if it
throws away the file it has read, only to reread it again, it's bloody
dumb.

Note that sequence of preprocessor tokens does not depend upon the ifdefs;
anything under #if 0 *is* tokenized all the same.  So it's not even that
"parsing" (tokenizing, actually) has to be repeated.

Re: [PATCH] proc: use #pragma once

[Rasmus Villemoes]

On 2018-05-04 00:42, Al Viro wrote:
> On Fri, May 04, 2018 at 12:14:57AM +0200, Rasmus Villemoes wrote:
> 
>> FWIW, it's not just removing some identifiers from cpp's hash tables, it
>> also reduces I/O: Due to our header mess, we have some cyclic includes,
>> e.g mm.h -> memremap.h -> mm.h. While parsing mm.h, cpp sees the #define
>> _LINUX_MM_H, then goes parsing memremap.h, but since it hasn't reached
>> the end of mm.h yet (seeing that there's nothing but comments outside
>> the #ifndef/#endif pair), it hasn't had a chance to set the internal
>> flag for mm.h, so it goes slurping in mm.h again. Obviously, the
>> definedness of _LINUX_MM_H at that point means it "only" has to parse
>> those 87K for comments and matching up #ifs, #ifdefs,#endifs etc. With
>> #pragma once, the flag gets set for mm.h immediately, so the #include
>> from memremap.h is entirely ignored. This can easily be verified with
>> strace. And mm.h is not the only header getting read twice.
> 
> Which gcc version it is?

When I stumbled on this a few years ago (I was stracing gcc for some
reason), 4.something. Verifying today, 5.4.

> Does it *really* read anything twice?

Yes, just do "strace -f" and grep 'open.*= [0-9]'.

> After all, you are going to read (and tokenize) the rest of mm.h anyway, so if it
> throws away the file it has read, only to reread it again, it's bloody
> dumb.

Hm, yes, gcc should be able to hang the buffer off of the structure it
keeps around anyway for tracking the status of mm.h. But it doesn't.

> Note that sequence of preprocessor tokens does not depend upon the ifdefs;
> anything under #if 0 *is* tokenized all the same.  So it's not even that
> "parsing" (tokenizing, actually) has to be repeated.

Are you sure about that? Maybe formally it has to (if the phases are
done strictly sequentially), but can't it go into some fast-forward mode
under #if 0 (or #ifndef $some-defined-macro) where it only looks for
string and char literals, comments and "control-flow" directives?
libcpp/internal.h does have

  /* True if we are skipping a failed conditional group.  */
  unsigned char skipping;

Rasmus

Re: [PATCH] proc: use #pragma once

[Al Viro]

On Fri, May 04, 2018 at 01:23:12AM +0200, Rasmus Villemoes wrote:
> > Note that sequence of preprocessor tokens does not depend upon the ifdefs;
> > anything under #if 0 *is* tokenized all the same.  So it's not even that
> > "parsing" (tokenizing, actually) has to be repeated.
> 
> Are you sure about that? Maybe formally it has to (if the phases are
> done strictly sequentially), but can't it go into some fast-forward mode
> under #if 0 (or #ifndef $some-defined-macro) where it only looks for
> string and char literals, comments and "control-flow" directives?

What else do you expect preprocessor tokenizer to do?  It must take care
of line continuations, it must recognize and skip whitespace (including
comments) and if it recognizes string and character literals, what else
is left to do?  pp-numbers?  Sure - that'd be
	[.]?[0-9]([0-9a-zA-Z_.]|[epEP][-+])*
Horribly hard to recognize, that...  Identifiers are no harder, obviously
(especially since keywords are _not_ something special at that level)
and punctuators are also easily taken care of.

Preprocessor tokens are nowhere near as much work as C ones.  And you
can't do mapping to C ones before the preprocessing is done - if nothing
else, quoting and token concatenation needs to happen first.