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

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.

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!

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

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.
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!

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

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!

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

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

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@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@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

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@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@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@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.

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.

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

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.
participants (4)
-
Dr. Philipp Tomsich
-
Måns Rullgård
-
Tom Rini
-
Wolfgang Denk