PSA: Google C++ style guide updated

[Victor Costan]

The C++ style guide was synced with the internal version. Diff here.

The content changes are fairly minor, but I figured it's worth notifying the folks who like to stay up to date.

    Victor

[dan...@chromium.org]

Thank you!

This is one interesting change.

<p>Prefer to use a <code>struct</code> instead of a pair or a
tuple whenever the elements can have meaningful names.</p>

<p>
  While using pairs and tuples can avoid the need to define a custom type,
  potentially saving work when <em>writing</em> code, a meaningful field
  name will almost always be much clearer when <em>reading</em> code than
  <code>.first</code>, <code>.second</code>, or <code>std::get&lt;X&gt;</code>.
  While C++14's introduction of <code>std::get&lt;Type&gt;</code> to access a
  tuple element by type rather than index (when the type is unique) can
  sometimes partially mitigate this, a field name is usually substantially
  clearer and more informative than a type.
</p>

<p>
  Pairs and tuples may be appropriate in generic code where there are not
  specific meanings for the elements of the pair or tuple. Their use may
  also be required in order to interoperate with existing code or APIs.

</p>

And this one may come up:

  <li>For a function parameter passed by value, <code>const</code> has
  no effect on the caller, thus is not recommended in function
  declarations. See <a href="https://abseil.io/tips/109">TotW #109</a>.

As well as the removal of the request to use `0.0` instead of `0` for real numbers.

- <p>Use <code>0</code> for integers, <code>0.0</code> for reals,

But is replaced by some new guidance:

<p>Floating-point literals should always have a radix point, with digits on both
sides, even if they use exponential notation. Readability is improved if all
floating-point literals take this familiar form, as this helps ensure that they
are not mistaken for integer literals, and that the
<code>E</code>/<code>e</code> of the exponential notation is not mistaken for a
hexadecimal digit. It is fine to initialize a floating-point variable with an
integer literal (assuming the variable type can exactly represent that integer),
but note that a number in exponential notation is never an integer literal.
</p>

<pre class="badcode">float f = 1.f;
long double ld = -.5L;
double d = 1248e6;
</pre>

<pre class="goodcode">float f = 1.0f;
float f2 = 1;   // Also OK
long double ld = -0.5L;
double d = 1248.0e6;

This advice is new:

Generally speaking, descriptiveness should be
proportional to the name's scope of visibility. For example,
<code>n</code> may be a fine name within a 5-line function,
but within the scope of a class, it's likely too vague.</p>

Advice to use DEPRECIATED comments is gone. We don't generally use these anyhow AFAIK.

- <p>Mark deprecated interface points with <code>DEPRECATED</code>
comments.</p>

- <p>You can mark an interface as deprecated by writing a
- comment containing the word <code>DEPRECATED</code> in
- all caps. The comment goes either before the declaration
- of the interface or on the same line as the
- declaration.</p>

That's what I found in the diff :)

Dana

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAP_mGKoYzjo9rtZ0e3gDnsWPQMgwnjW5svVSpJUD7B0o9g1ZYw%40mail.gmail.com.

[K Moon]

A couple other things I noticed:

This sentence was dropped (so related files no longer have an exception to the "include what you use" (IWYU) rule):
"However, any includes present in the related header do not need to be included again in the related <code>cc</code> (i.e., <code>foo.cc</code> can rely on <code>foo.h</code>'s includes)."

The ban on creating user-defined literals (UDLs) was expanded to using any UDLs. Among other diffs:
"Do not create user-defined literals." became "Do not use user-defined literals." A later section clarifies this includes "user"-defined literals in the standard library.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAHtyhaQWMygNXLR%3DT1Zf2fz%2BXRGEkA71G_qJdXvSMcYuVSYxPQ%40mail.gmail.com.

[Peter Kasting]

On Wed, Sep 4, 2019 at 10:22 AM K Moon <km...@chromium.org> wrote:

The ban on creating user-defined literals (UDLs) was expanded to using any UDLs. Among other diffs:
"Do not create user-defined literals." became "Do not use user-defined literals." A later section clarifies this includes "user"-defined literals in the standard library.

Interesting.  That makes things like <chrono> more annoying to use, which is irrelevant right now, but may become relevant later.

PK

[Peter Kasting]

On Wed, Sep 4, 2019 at 10:02 AM <dan...@chromium.org> wrote:

<p>Prefer to use a <code>struct</code> instead of a pair or a
tuple whenever the elements can have meaningful names.</p>

I think this obviates the need for this section of our Dos and Donts page: https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++-dos-and-donts.md#prefer-structs-over-pairs_tuples-when-used-repeatedly

Any objection to me removing it?

PK 

[dan...@chromium.org]

Yeah, I agree. Thanks!

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFBtPnjKOqC0OS1Su5MDeDUqMs0%3DE8XptRwQpc%2BUoSEyWA%40mail.gmail.com.

[Jan Wilken Dörrie]

Thank you very much! 

Considering that the internal style guide just yesterday was significantly updated again (C++17 is now allowed), would it be possible to perform another sync? 

E.g. it would be nice to be able to reference the guidance on lambda capture expressions again, which we recently removed from https://chromium-cpp.appspot.com/ in the process of default allowing C++14.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAHtyhaTPd%3Dw-98VrNBewsoLr-_LHPuGAw6ZdBygcLH2Jg5Owbg%40mail.gmail.com.

[Victor Costan]

On Thu, Sep 5, 2019 at 6:41 PM Jan Wilken Dörrie <jdoe...@chromium.org> wrote:

Considering that the internal style guide just yesterday was significantly updated again (C++17 is now allowed), would it be possible to perform another sync? 

Yup, I'll try to have that done soon.

    Victor 

[Nico Weber]

C++17 being allowed internally means we must be careful bout syncing, since PNaCl means we can't use C++17 for a while in Chromium.

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAP_mGKqRu%3DiQEsfao60TT_G-RVs5_NiHF_BQWXkn1aO6UJ3DwA%40mail.gmail.com.

[Victor Costan]

On Tue, Sep 10, 2019 at 10:17 AM Nico Weber <tha...@chromium.org> wrote:

C++17 being allowed internally means we must be careful bout syncing, since PNaCl means we can't use C++17 for a while in Chromium.

Apologies for being obtuse here. Do you mean that I shouldn't push for the style guide to get synced, or do you mean I should block syncs?

"careful about syncing" makes me think that we may want a snapshot of the guide as-is right now, before the C++17 changes get added. We'd then refer to that snapshot from our own docs, until we're able to switch to C++17.

Thanks in advance for the clarification,

    Victor

[Nico Weber]

On Mon, Sep 9, 2019 at 10:10 PM Victor Costan <pwn...@chromium.org> wrote:

On Tue, Sep 10, 2019 at 10:17 AM Nico Weber <tha...@chromium.org> wrote:

C++17 being allowed internally means we must be careful bout syncing, since PNaCl means we can't use C++17 for a while in Chromium.

Apologies for being obtuse here. Do you mean that I shouldn't push for the style guide to get synced, or do you mean I should block syncs?

"careful about syncing" makes me think that we may want a snapshot of the guide as-is right now, before the C++17 changes get added. We'd then refer to that snapshot from our own docs, until we're able to switch to C++17.

That's a great question! It probably deserves its own thread on cxx@. I'm guessing snapshotting is probably the simplest thing to do.

 

Thanks in advance for the clarification,

    Victor

On Mon, Sep 9, 2019 at 8:18 PM Victor Costan <pwn...@chromium.org> wrote:

On Thu, Sep 5, 2019 at 6:41 PM Jan Wilken Dörrie <jdoe...@chromium.org> wrote:

Considering that the internal style guide just yesterday was significantly updated again (C++17 is now allowed), would it be possible to perform another sync? 

Yup, I'll try to have that done soon.

    Victor 

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAP_mGKqRu%3DiQEsfao60TT_G-RVs5_NiHF_BQWXkn1aO6UJ3DwA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAP_mGKozRQntuSWXREhx3%3D_aMYsUz7hUfx2_uY9-uyefMjELSw%40mail.gmail.com.

[Jan Wilken Dörrie]

FWIW the internal style guide already has the C++17 changes added, so I believe the version that is currently externally available is effectively the most up to date version before those changes got added.

However, I don't completely understand why us not being able to use C++17 yet hinders us from following the updated style guide in general. Admittedly, the advice on structured bindings or class template argument deduction is not yet relevant for us, but it doesn't hurt us either, right? Since the updated guide also includes guidance around features that only got added in C++14 (e.g. decltype(auto), generic lambdas, lambda init captures, etc), which we are able to use, trying to make the latest internal changes externally available will be of value for us.

Best,

Jan