Page MenuHomeFreeBSD

style: Allow C++ comments
ClosedPublic

Authored by imp on Jul 26 2022, 9:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 9, 6:15 AM
Unknown Object (File)
Wed, Oct 23, 9:54 AM
Unknown Object (File)
Oct 7 2024, 11:38 PM
Unknown Object (File)
Sep 30 2024, 11:25 PM
Unknown Object (File)
Sep 27 2024, 10:24 AM
Unknown Object (File)
Sep 27 2024, 10:19 AM
Unknown Object (File)
Sep 23 2024, 4:23 PM
Unknown Object (File)
Sep 21 2024, 7:06 PM

Details

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 46613
Build 43502: arc lint + arc unit

Event Timeline

imp requested review of this revision.Jul 26 2022, 9:02 PM
brooks added inline comments.
share/man/man9/style.9
67

I'd prefer to encourage per-file consistency. In new code I'd see no reason not to permit // multi-line comments.

I've seen patches where BCPL-style comments are abused to form what we're used to see as block comments. So if we start allowing them, maybe say that they're allowed under some conditions (not sure what to specify here or how).

👍

share/man/man9/style.9
67

I'm not sure "either style" reads correctly here, since we really have four styles above: VERY important single-line, single-line, other single-line, multi-line. I think what we really mean is either /* */ or // style [independent of single or multi-line].

IIRC we have a general "conform to the existing style" statement that would suggest not adding one // comment in a file that's currently exclusively /* */

share/man/man9/style.9
67

Our comments crossed paths, but that's what I was trying to get at. Perhaps it's worth adding a brief reference to that here.

I would suggest clarifying "appropriate for the situation".

I would say:

  1. Only in C++ files, not C files. (Unless the community decides otherwise)
  2. Match the nearby context.

Also, this specifically calls out single-line comments. Are comment-blocks going to continue to be C-style or are we going to start allowing C++-style too?

Clarify my rather vague language based on the questions so far

share/man/man9/style.9
67–68

C++ comments should only be used for single line comments, or comments at the

end of a statement.
Single line comments don't necessarily follow a statement.

Perhaps I'm too nitpicky, but I think the fewer implied rules, the better. So I would mention that:
Single line PL/I-style comments are required in existing code, or in new code where they don't end at the end of the line. Otherwise it's allowed to use a BCPL-style comment.

Mmm, I'm not quite sure how I feel about mixing comment styles. That is, mandating block comments still use /* while single line in the same file uses // even for a new file seems kind of odd. I feel like a given file should either be one or the other including block comments like license statements, etc. I think you'd end up with something like this for when a file uses // comments:

//
// VERY important single-line comments look like this.
//

// Most single-line comments look like this.

// Multi-line comments look like this.  Make them real sentences.  Fill
// them so they look like real paragraphs.

And then for the copyright header:

//-
// SPDX...
//

But I'm leery of using this in C files given how prevalent /* comments are in our existing code base IMO.

FWIW in GDB where it's pretty much full-blown C++11 now (RAII, lambdas, smart pointers, templates, etc.) it's still /* comments.

pauamma_gundo.com added inline comments.
share/man/man9/style.9
67

Is "C++ comments" the accepted name among C developers for // ... \n comments? Calling them that instead of say, "C++-style comments" always makes me wonder when FreeBSD started using C++.

share/man/man9/style.9
67

"C++ style comments" is better. The Wikipedia article on C99 says "support for one-line comments beginning with , as in BCPL, C++ and Java." Some other articles just use a phrase like "single line comments beginning with "

I don't think we want to specify // as allowable for single line comments but require /* */ for multi-line comments as mixing them is worse.

/*
 * A multi-line
 * comment like this
 */

// and a single line comment like this

vs

// A multi-line comment
// like this

// and a single line comment like this

That said FreeBSD does use C++ already for some things, e.g. devd and Clang. (And, now that I look sbin/devd/devd.c has a somewhat unfortunate mix of /* */ and // for both multi-line and single-line comments.)

share/man/man9/style.9
67

That should be "support for one-line comments beginning with //, as in BCPL, C++ and Java"

Hi,

How about:
C++ comments may be used in C++ code in FreeBSD?

I know at least "indent" in base doesn't handle C++ comments very well:

Example how a semicolon after // confuses indent:

# cat << EOF | indent
? // my comment ; is now suddenly no longer code
? EOF
// my comment;
is		now suddenly no longer code

--HPS

I know at least "indent" in base doesn't handle C++ comments very well:

Example how a semicolon after // confuses indent:

# cat << EOF | indent
? // my comment ; is now suddenly no longer code
? EOF
// my comment;
is		now suddenly no longer code

That has been known. Support for BCPL-style comments has never been implemented in BSD indent, except recently in NetBSD indent which is a fork of our indent.

rgrimes added a subscriber: rgrimes.

With the constraints that are here I am in favor of allowing these.

This revision is now accepted and ready to land.Jul 28 2022, 3:03 PM
share/man/man9/style.9
69

For consistency, use the same name as above (whether my suggestion, emaste's, or whatever it ends up being).

What about just:

Comments starting with // may be used in both C and C++.
Styles should not be mixed within a file or component.

What about just:

Comments starting with // may be used in both C and C++.
Styles should not be mixed within a file or component.

Define "component"?

Define "component"?

A module, group of related files, etc., like a single userland utility (say bin/ls), a device driver, etc.

Define "component"?

A module, group of related files, etc., like a single userland utility (say bin/ls), a device driver, etc.

Audience check: is making the clarification explicit needed?

This revision was automatically updated to reflect the committed changes.

For the record, I think this landed in a good place avoiding a free-for-all in every file while letting people writing large pieces of code do what they want.