[U-Boot] GCC 7.x vs. C++ comments

[U-Boot] GCC 7.x vs. C++ comments

[Dr. Philipp Tomsich]

Tom,

I recently ran a local buildman with a came across these:
>   cc -Wp,-MD,tools/.gen_eth_addr.d -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer    -include ./include/compiler.h -idirafterinclude -idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools -DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE -pedantic -o tools/gen_eth_addr tools/gen_eth_addr.c  
> tools/gen_eth_addr.c:1:1: warning: C++ style comments are not allowed in ISO C90
>  // SPDX-License-Identifier: GPL-2.0+
>  ^
> tools/gen_eth_addr.c:1:1: warning: (this will be reported only once per input file)
>   cc -Wp,-MD,tools/.gen_ethaddr_crc.o.d -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer    -include ./include/compiler.h -idirafterinclude -idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools -DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE -pedantic -c -o tools/gen_ethaddr_crc.o tools/gen_ethaddr_crc.c
> tools/gen_ethaddr_crc.c:1:1: warning: C++ style comments are not allowed in ISO C90
>  // SPDX-License-Identifier: GPL-2.0+
>  ^
> tools/gen_ethaddr_crc.c:1:1: warning: (this will be reported only once per input file)
>   echo "#include <../lib/crc8.c>" >tools/lib/crc8.c
>   cc -Wp,-MD,tools/lib/.crc8.o.d -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer    -include ./include/compiler.h -idirafterinclude -idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools -DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE -pedantic -c -o tools/lib/crc8.o tools/lib/crc8.c
> In file included from tools/lib/crc8.c:1:0:
> ./tools/../lib/crc8.c:1:1: warning: C++ style comments are not allowed in ISO C90
>  // SPDX-License-Identifier: GPL-2.0+
>  ^

The system compiler was a
> Using built-in specs.
> COLLECT_GCC=cc
> COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
> Target: x86_64-linux-gnu
> Configured with: ../src/configure -v --with-pkgversion='Debian 4.9.2-10+deb8u1' --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.9 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --with-arch-32=i586 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
> Thread model: posix
> gcc version 4.9.2 (Debian 4.9.2-10+deb8u1) 


What’s your preferred solution:
	(a) change these comments
	(b) change our Makefiles to let GCC know that we are compiling gnu99/C99?

Neither solution is too appealing to me, so I am asking...

Thanks,
Philipp.

[U-Boot] GCC 7.x vs. C++ comments

[Tom Rini]

On Wed, May 09, 2018 at 10:46:10AM +0200, Dr. Philipp Tomsich wrote:
> Tom,
> 
> I recently ran a local buildman with a came across these:
> >   cc -Wp,-MD,tools/.gen_eth_addr.d -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer    -include ./include/compiler.h -idirafterinclude -idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools -DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE -pedantic -o tools/gen_eth_addr tools/gen_eth_addr.c  
> > tools/gen_eth_addr.c:1:1: warning: C++ style comments are not allowed in ISO C90
> >  // SPDX-License-Identifier: GPL-2.0+
> >  ^
> > tools/gen_eth_addr.c:1:1: warning: (this will be reported only once per input file)
> >   cc -Wp,-MD,tools/.gen_ethaddr_crc.o.d -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer    -include ./include/compiler.h -idirafterinclude -idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools -DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE -pedantic -c -o tools/gen_ethaddr_crc.o tools/gen_ethaddr_crc.c
> > tools/gen_ethaddr_crc.c:1:1: warning: C++ style comments are not allowed in ISO C90
> >  // SPDX-License-Identifier: GPL-2.0+
> >  ^
> > tools/gen_ethaddr_crc.c:1:1: warning: (this will be reported only once per input file)
> >   echo "#include <../lib/crc8.c>" >tools/lib/crc8.c
> >   cc -Wp,-MD,tools/lib/.crc8.o.d -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer    -include ./include/compiler.h -idirafterinclude -idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools -DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE -pedantic -c -o tools/lib/crc8.o tools/lib/crc8.c
> > In file included from tools/lib/crc8.c:1:0:
> > ./tools/../lib/crc8.c:1:1: warning: C++ style comments are not allowed in ISO C90
> >  // SPDX-License-Identifier: GPL-2.0+
> >  ^
> 
> The system compiler was a
> > Using built-in specs.
> > COLLECT_GCC=cc
> > COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
> > Target: x86_64-linux-gnu
> > Configured with: ../src/configure -v --with-pkgversion='Debian 4.9.2-10+deb8u1' --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.9 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --with-arch-32=i586 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
> > Thread model: posix
> > gcc version 4.9.2 (Debian 4.9.2-10+deb8u1) 
> 
> 
> What’s your preferred solution:
> 	(a) change these comments
> 	(b) change our Makefiles to let GCC know that we are compiling gnu99/C99?
> 
> Neither solution is too appealing to me, so I am asking...

Lets go with (b) which I guess what a more modern toolchain defaults to
anyhow.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180509/9ced0f1c/attachment.sig>

[U-Boot] GCC 7.x vs. C++ comments

[Wolfgang Denk]

Dear Tom,

In message <20180509111627.GD12235@bill-the-cat.ec.rr.com> you wrote:
> 
> > What’s your preferred solution:
> > 	(a) change these comments
> > 	(b) change our Makefiles to let GCC know that we are compiling gnu99/C99?
> > 
> > Neither solution is too appealing to me, so I am asking...
>
> Lets go with (b) which I guess what a more modern toolchain defaults to
> anyhow.  Thanks!

I disagree.  UI-Boot always discouraged the use of C++ comments, see
for example bullet # 5 at [1]. We should clearly fix the comments,
please.

[1] http://www.denx.de/wiki/U-Boot/Coding

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"It takes all sorts of in & out-door schooling to get adapted  to  my
kind of fooling"                                           - R. Frost

[U-Boot] GCC 7.x vs. C++ comments

[Tom Rini]

On Wed, May 09, 2018 at 01:38:37PM +0200, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20180509111627.GD12235@bill-the-cat.ec.rr.com> you wrote:
> > 
> > > What’s your preferred solution:
> > > 	(a) change these comments
> > > 	(b) change our Makefiles to let GCC know that we are compiling gnu99/C99?
> > > 
> > > Neither solution is too appealing to me, so I am asking...
> >
> > Lets go with (b) which I guess what a more modern toolchain defaults to
> > anyhow.  Thanks!
> 
> I disagree.  UI-Boot always discouraged the use of C++ comments, see
> for example bullet # 5 at [1]. We should clearly fix the comments,
> please.
> 
> [1] http://www.denx.de/wiki/U-Boot/Coding

We should go and update [1] to note some special exemptions to the rule.
I see you missed out on the SPDX thread over here:
https://lists.denx.de/pipermail/u-boot/2018-May/327544.html and repeat
myself, I see it as more worthwhile to (a) follow the kernel in this
area (for both tooling and consistency and ease of development for our
overlapping community) (b) save space (in just about every conversion we
went from 2 lines to 1 line).  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180509/9ff5f0cf/attachment.sig>

[U-Boot] GCC 7.x vs. C++ comments

[Wolfgang Denk]

Dear Tom,

In message <20180509114828.GG12235@bill-the-cat.ec.rr.com> you wrote:
> 
> We should go and update [1] to note some special exemptions to the rule.

I'm not happy about this.

> I see you missed out on the SPDX thread over here:
> https://lists.denx.de/pipermail/u-boot/2018-May/327544.html and repeat

Marek already said what was on my mind, and got ignored.
Would it have changed anything if I had posted another complaint?

I'm doing now, and apparently I get ignored, too.  So what exactly
is your argument?

> myself, I see it as more worthwhile to (a) follow the kernel in this
> area (for both tooling and consistency and ease of development for our
> overlapping community) (b) save space (in just about every conversion we
> went from 2 lines to 1 line).  Thanks!

OK, so you decided, and any additional discussion is futile...

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It is practically impossible to teach good programming style to  stu-
dents that have had prior exposure to BASIC: as potential programmers
they are mentally mutilated beyond hope of regeneration.   - Dijkstra

[U-Boot] GCC 7.x vs. C++ comments

[Tom Rini]

On Wed, May 09, 2018 at 02:46:54PM +0200, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20180509114828.GG12235@bill-the-cat.ec.rr.com> you wrote:
> > 
> > We should go and update [1] to note some special exemptions to the rule.
> 
> I'm not happy about this.
> 
> > I see you missed out on the SPDX thread over here:
> > https://lists.denx.de/pipermail/u-boot/2018-May/327544.html and repeat
> 
> Marek already said what was on my mind, and got ignored.
> Would it have changed anything if I had posted another complaint?

Ignored, no.  Counts as a veto?  No.  And if you had chimed in too, I
don't know if that would have gotten anyone else to also chime in.
Looking over the thread again there's two yes votes, two no votes, two
people that chimed in on the thread but didn't express a yes or no to
the change, and then no one else has said anything.  The main thing I
see currently is a whole lot of ambivalence.

> I'm doing now, and apparently I get ignored, too.  So what exactly
> is your argument?
> 
> > myself, I see it as more worthwhile to (a) follow the kernel in this
> > area (for both tooling and consistency and ease of development for our
> > overlapping community) (b) save space (in just about every conversion we
> > went from 2 lines to 1 line).  Thanks!
> 
> OK, so you decided, and any additional discussion is futile...

It's not futile, but here's as best I can tell, the arguments:
Against Linux Kernel style SPDX tags:
- Don't like // style comments
- Visually inconsistent / jarring

For Linux Kernel style SPDX tags:
- Has higher visibility.
- Has tooling to enforce correctly formatted tags.
- Shorter (enforced as a single line comment means we don't have people
  spacing around it).
- Consistent expectations for our overlapping developer community.

Things that could be taken, without changing overall formatting:
- Logic operators for exceptions/dual-license/etc

If people speak up against the change now that we've done it, we could
revert and then add in the "LICENSE-A OR LICENSE-B" change.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180509/2d0f32e6/attachment.sig>

[U-Boot] GCC 7.x vs. C++ comments

[Dr. Philipp Tomsich]

> On 9 May 2018, at 15:49, Tom Rini <trini@konsulko.com> wrote:
> 
> On Wed, May 09, 2018 at 02:46:54PM +0200, Wolfgang Denk wrote:
>> Dear Tom,
>> 
>> In message <20180509114828.GG12235@bill-the-cat.ec.rr.com> you wrote:
>>> 
>>> We should go and update [1] to note some special exemptions to the rule.
>> 
>> I'm not happy about this.
>> 
>>> I see you missed out on the SPDX thread over here:
>>> https://lists.denx.de/pipermail/u-boot/2018-May/327544.html and repeat
>> 
>> Marek already said what was on my mind, and got ignored.
>> Would it have changed anything if I had posted another complaint?
> 
> Ignored, no.  Counts as a veto?  No.  And if you had chimed in too, I
> don't know if that would have gotten anyone else to also chime in.
> Looking over the thread again there's two yes votes, two no votes, two
> people that chimed in on the thread but didn't express a yes or no to
> the change, and then no one else has said anything.  The main thing I
> see currently is a whole lot of ambivalence.

Although I am ambivalent to the underlying discussion, I have strong
opinions regarding the language/standard-compliance...

My vote goes to C++ comments and upgrading the language standard
to C99 (or rather gnu99, as our code uses extensions).  This will (at least
somewhat) match how the default C compliance level in GCC has evolved
over the GCC 6 through GCC 8 release cycles.

And while we’re at it, we should allow "for (int i = 0; …”-style C99 declaration
of loop iterations within the loop-head.

> 
>> I'm doing now, and apparently I get ignored, too.  So what exactly
>> is your argument?
>> 
>>> myself, I see it as more worthwhile to (a) follow the kernel in this
>>> area (for both tooling and consistency and ease of development for our
>>> overlapping community) (b) save space (in just about every conversion we
>>> went from 2 lines to 1 line).  Thanks!
>> 
>> OK, so you decided, and any additional discussion is futile...
> 
> It's not futile, but here's as best I can tell, the arguments:
> Against Linux Kernel style SPDX tags:
> - Don't like // style comments
> - Visually inconsistent / jarring
> 
> For Linux Kernel style SPDX tags:
> - Has higher visibility.
> - Has tooling to enforce correctly formatted tags.
> - Shorter (enforced as a single line comment means we don't have people
>  spacing around it).
> - Consistent expectations for our overlapping developer community.
> 
> Things that could be taken, without changing overall formatting:
> - Logic operators for exceptions/dual-license/etc
> 
> If people speak up against the change now that we've done it, we could
> revert and then add in the "LICENSE-A OR LICENSE-B" change.  Thanks!
> 
> -- 
> Tom

[U-Boot] GCC 7.x vs. C++ comments

[Wolfgang Denk]

Dear Tom,

In message <20180509134905.GK12235@bill-the-cat.ec.rr.com> you wrote:
> 
> > Marek already said what was on my mind, and got ignored.
> > Would it have changed anything if I had posted another complaint?
>
> Ignored, no.  Counts as a veto?  No.  And if you had chimed in too, I
> don't know if that would have gotten anyone else to also chime in.

This does not really convince me.

IMO it makes no sense to blow up mailing list traffic with extensive
voting for such things. Marek said everything that needed to be
said, and repeating it would IMO not add any weight to it.  If you
really want a poll including more people, then explicitly start one
- but please not on the mailing list, at least not fuch such
details.

> It's not futile, but here's as best I can tell, the arguments:
> Against Linux Kernel style SPDX tags:

I think this argument is wrong from the start.  This is not about
"Linux Kernel style SPDX tags".  There is two different topics,
which are actually independent, and should be treated as such:

- Linux Kernel style SPDX tags, or rather more modern SPDX tags
  including the needed operators to deal with exceptions, multiple
  licenses, etc.  When I invented the SPDX tags I did not foresee
  this need (my fault), but I'm still proud that U-Boot introduced
  this mechanism at all.

  Yes, it is necessary to adapt the new developments in this area.

- Comment style.  This is just a matter of coding style and
  preferences.  Whether you use C comments (single line or
  multi-line) or C++ comments does not make any difference
  technically.

  U-Boot has been discouraging the use of C++ comments for 18 years,
  and I still see no good reason to change this.  [And yes, we also
  have the rule not to meddle with code copied from other projects.]

> - Don't like // style comments
> - Visually inconsistent / jarring

- Against existing coding style.


> For Linux Kernel style SPDX tags:
> - Has higher visibility.

??? I can't parse this.  In which way has

// SPDX-License-Identifier: GPL-2.0+

"higher visibility" than

/* SPDX-License-Identifier: GPL-2.0+ */

or even

/*
 * SPDX-License-Identifier: GPL-2.0+
 */

?

[IMO, the last form is the most visible one.]

And since when do we care about a single line of white space or two
when it comes to consistency or readability?

> - Has tooling to enforce correctly formatted tags.


> - Shorter (enforced as a single line comment means we don't have people
>   spacing around it).

Come on, this argument is really lame.

> - Consistent expectations for our overlapping developer community.

Please explain?  Who associates SPDX tags with C++ comments?  This
is silly.  We don't use these in Makefiles, or in shell scripts, or
in ...

And when talking about consistency - what about this in the current
Linux Kernel tree:

arch/x86/kernel/apic/apic_common.c: * SPDX-License-Identifier: GPL-2.0
arch/s390/tools/gen_opcode_table.c:/* SPDX-License-Identifier: GPL-2.0 */
arch/arm64/crypto/sha3-ce-glue.c:/* SPDX-License-Identifier: GPL-2.0 */
arch/arm64/crypto/sha512-ce-glue.c:/* SPDX-License-Identifier: GPL-2.0 */
arch/riscv/kernel/ftrace.c:/* SPDX-License-Identifier: GPL-2.0 */
arch/riscv/kernel/module-sections.c:/* SPDX-License-Identifier: GPL-2.0
drivers/tty/hvc/hvc_riscv_sbi.c:/* SPDX-License-Identifier: GPL-2.0 */
drivers/memory/brcmstb_dpfe.c: * SPDX-License-Identifier: GPL-2.0
drivers/soc/amlogic/meson-mx-socinfo.c: * SPDX-License-Identifier: GPL-2.0+
drivers/soc/amlogic/meson-gx-pwrc-vpu.c: * SPDX-License-Identifier: GPL-2.0+
drivers/soc/amlogic/meson-gx-socinfo.c: * SPDX-License-Identifier: GPL-2.0+
drivers/i2c/busses/i2c-sprd.c: * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c: * SPDX-License-Identifier: (GPL-2.0+ or MIT)
drivers/pinctrl/meson/pinctrl-meson-axg.c: * SPDX-License-Identifier: (GPL-2.0+ or MIT)
drivers/virt/vboxguest/vboxguest_core.c:/* SPDX-License-Identifier: (GPL-2.0 OR CDDL-1.0) */
drivers/virt/vboxguest/vboxguest_linux.c:/* SPDX-License-Identifier: GPL-2.0 */
drivers/virt/vboxguest/vboxguest_utils.c:/* SPDX-License-Identifier: (GPL-2.0 OR CDDL-1.0) */
drivers/watchdog/rtd119x_wdt.c: * SPDX-License-Identifier: GPL-2.0+
drivers/rtc/rtc-rtd119x.c: * SPDX-License-Identifier: GPL-2.0+
drivers/rtc/rtc-sc27xx.c: * SPDX-License-Identifier: GPL-2.0
...


Yes, 47 files is only a small fraction compared against the 5261
C files with C++ commented tags. But consistency of apparently not a
real issue when it comes to comment style in Linux.


> Things that could be taken, without changing overall formatting:
> - Logic operators for exceptions/dual-license/etc

Right, this is completely independent and out of question here.

> If people speak up against the change now that we've done it, we could
> revert and then add in the "LICENSE-A OR LICENSE-B" change.  Thanks!

I have always been against the use of C++ comments in U-Boot (and in
general in non C++ code), and I still am against it.

Not that this matters much.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The rule on staying alive as a program manager is to give 'em a  num-
ber or give 'em a date, but never give 'em both at once.

[U-Boot] GCC 7.x vs. C++ comments

[Wolfgang Denk]

Dear Tom,

In message <20180509154052.5E0B424000A@gemini.denx.de> I wrote:
> 
> > - Don't like // style comments
> > - Visually inconsistent / jarring
> 
> - Against existing coding style.

Also, the SPDX tag is rarely a separate comment line.  In most
cases, it is part of a larger file header, say for example:

common/main.c:

/*
 * (C) Copyright 2000
 * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
 *
 * SPDX-License-Identifier:     GPL-2.0+
 */


Do you suggest to reformat this into something like:

/*
 * (C) Copyright 2000
 * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
 */

// SPDX-License-Identifier:     GPL-2.0+

?

If yes, then please explain which sense this would make?  It is just
unnecessay work, and the result is inconsistent and ugly.


> > - Has tooling to enforce correctly formatted tags.

I forgot to ask which "tooling" you have in mind here?  I did not
see anything like that in the kernel source tree.  What am I
missing?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
This restaurant was advertising breakfast  any  time.  So  I  ordered
french toast in the renaissance.            - Steven Wright, comedian

[U-Boot] GCC 7.x vs. C++ comments

[Tom Rini]

On Wed, May 09, 2018 at 06:07:50PM +0200, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20180509154052.5E0B424000A@gemini.denx.de> I wrote:
> > 
> > > - Don't like // style comments
> > > - Visually inconsistent / jarring
> > 
> > - Against existing coding style.
> 
> Also, the SPDX tag is rarely a separate comment line.  In most
> cases, it is part of a larger file header, say for example:
> 
> common/main.c:
> 
> /*
>  * (C) Copyright 2000
>  * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>  *
>  * SPDX-License-Identifier:     GPL-2.0+
>  */
> 
> 
> Do you suggest to reformat this into something like:
> 
> /*
>  * (C) Copyright 2000
>  * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>  */
> 
> // SPDX-License-Identifier:     GPL-2.0+
> 
> ?

I know it's going to annoy you more, but yes, that's already _done_:
$ head -n5 common/main.c
// SPDX-License-Identifier: GPL-2.0+
/*
 * (C) Copyright 2000
 * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
 */

It was about 97% automatic perl regex + sed insert and 3% "Ugh, this
file does not follow the normal conventional comment style at all".

> If yes, then please explain which sense this would make?  It is just
> unnecessay work, and the result is inconsistent and ugly.
> 
> 
> > > - Has tooling to enforce correctly formatted tags.
> 
> I forgot to ask which "tooling" you have in mind here?  I did not
> see anything like that in the kernel source tree.  What am I
> missing?

This started because I updated checkpatch.pl and that in turn checks if
new files have an SPDX tag and if so, does it match the kernel style
formatting.  The first email:
https://lists.denx.de/pipermail/u-boot/2018-April/325510.html
that brought this up.  And yes, I run checkpatch.pl on everything before
every pull/push.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180509/5689e3ca/attachment.sig>

[U-Boot] GCC 7.x vs. C++ comments

[Tom Rini]

On Wed, May 09, 2018 at 05:40:52PM +0200, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20180509134905.GK12235@bill-the-cat.ec.rr.com> you wrote:
> > 
> > > Marek already said what was on my mind, and got ignored.
> > > Would it have changed anything if I had posted another complaint?
> >
> > Ignored, no.  Counts as a veto?  No.  And if you had chimed in too, I
> > don't know if that would have gotten anyone else to also chime in.
> 
> This does not really convince me.
> 
> IMO it makes no sense to blow up mailing list traffic with extensive
> voting for such things. Marek said everything that needed to be
> said, and repeating it would IMO not add any weight to it.  If you
> really want a poll including more people, then explicitly start one
> - but please not on the mailing list, at least not fuch such
> details.

I honestly couldn't think of a better way to see if anyone cared besides
an RFC to the mailing list.

> > It's not futile, but here's as best I can tell, the arguments:
> > Against Linux Kernel style SPDX tags:
> 
> I think this argument is wrong from the start.  This is not about
> "Linux Kernel style SPDX tags".  There is two different topics,
> which are actually independent, and should be treated as such:
> 
> - Linux Kernel style SPDX tags, or rather more modern SPDX tags
>   including the needed operators to deal with exceptions, multiple
>   licenses, etc.  When I invented the SPDX tags I did not foresee
>   this need (my fault), but I'm still proud that U-Boot introduced
>   this mechanism at all.
> 
>   Yes, it is necessary to adapt the new developments in this area.

And for the record, it's a good thing you did.  Since we were the best
example of a project going all-in on the tags for a long while I know
the people that did the kernel scheme looked at what we had.

> - Comment style.  This is just a matter of coding style and
>   preferences.  Whether you use C comments (single line or
>   multi-line) or C++ comments does not make any difference
>   technically.

I disagree on these being separate.  I copy/pasted the relevant part of
the kernel documentation as an update to our doc but where it goes
(first line) and how it goes (comments like ....) are as much a part of
the style as the syntax.  I'd argue that where the license copies go and
some directory structure there-of is also part of it but I think our
locations are more set in stone, but we can live with it.

>   U-Boot has been discouraging the use of C++ comments for 18 years,
>   and I still see no good reason to change this.  [And yes, we also
>   have the rule not to meddle with code copied from other projects.]

I don't want to have it buried here but maybe it's time to talk about
fully adopting C99 (or, GNU C99).  Did you happen to read
https://lkml.org/lkml/2017/11/25/133 that Yamada-san passed along?
Having read that after converting the tags that my first regex missed,
maybe we were wrong 18 years ago.

> > - Don't like // style comments
> > - Visually inconsistent / jarring
> 
> - Against existing coding style.
> 
> 
> > For Linux Kernel style SPDX tags:
> > - Has higher visibility.
> 
> ??? I can't parse this.  In which way has
> 
> // SPDX-License-Identifier: GPL-2.0+
> 
> "higher visibility" than
> 
> /* SPDX-License-Identifier: GPL-2.0+ */
> 
> or even

I was pointing out first line vs somewhere within the top comment block
as I don't consider comment format vs location different items.

> /*
>  * SPDX-License-Identifier: GPL-2.0+
>  */
> 
> ?
> 
> [IMO, the last form is the most visible one.]
> 
> And since when do we care about a single line of white space or two
> when it comes to consistency or readability?
> 
> > - Has tooling to enforce correctly formatted tags.
> 
> 
> > - Shorter (enforced as a single line comment means we don't have people
> >   spacing around it).
> 
> Come on, this argument is really lame.

That the SPDX tag meant the same as the whole license text was part of
the reason to convert.

> > - Consistent expectations for our overlapping developer community.
> 
> Please explain?  Who associates SPDX tags with C++ comments?  This

This is again part of the difference in counting comment format as part
of it, or not.

> is silly.  We don't use these in Makefiles, or in shell scripts, or
> in ...

We sometimes do for Makefiles, almost always do in shell scripts.

> And when talking about consistency - what about this in the current
> Linux Kernel tree:
> 
> arch/x86/kernel/apic/apic_common.c: * SPDX-License-Identifier: GPL-2.0
> arch/s390/tools/gen_opcode_table.c:/* SPDX-License-Identifier: GPL-2.0 */
> arch/arm64/crypto/sha3-ce-glue.c:/* SPDX-License-Identifier: GPL-2.0 */
> arch/arm64/crypto/sha512-ce-glue.c:/* SPDX-License-Identifier: GPL-2.0 */
> arch/riscv/kernel/ftrace.c:/* SPDX-License-Identifier: GPL-2.0 */
> arch/riscv/kernel/module-sections.c:/* SPDX-License-Identifier: GPL-2.0
> drivers/tty/hvc/hvc_riscv_sbi.c:/* SPDX-License-Identifier: GPL-2.0 */
> drivers/memory/brcmstb_dpfe.c: * SPDX-License-Identifier: GPL-2.0
> drivers/soc/amlogic/meson-mx-socinfo.c: * SPDX-License-Identifier: GPL-2.0+
> drivers/soc/amlogic/meson-gx-pwrc-vpu.c: * SPDX-License-Identifier: GPL-2.0+
> drivers/soc/amlogic/meson-gx-socinfo.c: * SPDX-License-Identifier: GPL-2.0+
> drivers/i2c/busses/i2c-sprd.c: * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c: * SPDX-License-Identifier: (GPL-2.0+ or MIT)
> drivers/pinctrl/meson/pinctrl-meson-axg.c: * SPDX-License-Identifier: (GPL-2.0+ or MIT)
> drivers/virt/vboxguest/vboxguest_core.c:/* SPDX-License-Identifier: (GPL-2.0 OR CDDL-1.0) */
> drivers/virt/vboxguest/vboxguest_linux.c:/* SPDX-License-Identifier: GPL-2.0 */
> drivers/virt/vboxguest/vboxguest_utils.c:/* SPDX-License-Identifier: (GPL-2.0 OR CDDL-1.0) */
> drivers/watchdog/rtd119x_wdt.c: * SPDX-License-Identifier: GPL-2.0+
> drivers/rtc/rtc-rtd119x.c: * SPDX-License-Identifier: GPL-2.0+
> drivers/rtc/rtc-sc27xx.c: * SPDX-License-Identifier: GPL-2.0
> ...
> 
> 
> Yes, 47 files is only a small fraction compared against the 5261
> C files with C++ commented tags. But consistency of apparently not a
> real issue when it comes to comment style in Linux.

Inconsistent rollout?  Tags went in good bit before the document that
said how/where they should be was finally merged.  I'm sure it's on the
kernel janitors list for someone to fixup, or there's patches slowly
in-flight to do so.

> > Things that could be taken, without changing overall formatting:
> > - Logic operators for exceptions/dual-license/etc
> 
> Right, this is completely independent and out of question here.

Agreed.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180509/8a8c4ef7/attachment.sig>

[U-Boot] GCC 7.x vs. C++ comments

[Wolfgang Denk]

Dear Tom,

In message <20180509161456.GM12235@bill-the-cat.ec.rr.com> you wrote:
> 
> I don't want to have it buried here but maybe it's time to talk about
> fully adopting C99 (or, GNU C99).  Did you happen to read
> https://lkml.org/lkml/2017/11/25/133 that Yamada-san passed along?
> Having read that after converting the tags that my first regex missed,
> maybe we were wrong 18 years ago.

OK.  You know my opinion.

> > is silly.  We don't use these in Makefiles, or in shell scripts, or
> > in ...
>
> We sometimes do for Makefiles, almost always do in shell scripts.

You misunderstand.  I meant: we do not use C++ comments in Makefiles
or in shell scripts, or in most other non-C code...

I drop out of this discussion here - thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Ada is PL/I trying to be Smalltalk.                 - Codoso diBlini

[U-Boot] GCC 7.x vs. C++ comments

[Måns Rullgård]

Tom Rini <trini@konsulko.com> writes:

> - Shorter

Please explain how

  // SPDX

is significantly shorter than

  /* SPDX */

Having // comments next to /* ones is just hideously ugly.  If some
scanning tool is too stupid to handle standard C comments, it's the tool
that needs to be fixed.

-- 
Måns Rullgård