Re: [chromium-dev] C++ code style: Should we rely on "transitive inclusions" from an interface

[K. Moon]

Probably more of a cxx@ question than a general chromium-dev@ question, but here's my take...

I wouldn't include "a.h" in "c.h", but I probably would include it in "c.cc". I'm sure other people have different opinions on this, though; the relevant style guide text isn't specific about this.

Here's what the style guide says:

If a source or header file refers to a symbol defined elsewhere, the file should directly include a header file which properly intends to provide a declaration or definition of that symbol. It should not include header files for any other reason.

Do not rely on transitive inclusions. This allows people to remove no-longer-needed #include statements from their headers without breaking clients. This also applies to related headers - foo.cc should include bar.h if it uses a symbol from it even if foo.h includes bar.h.

https://google.github.io/styleguide/cppguide.html#Include_What_You_Use

The weasel phrase here is, "properly intends to provide," because it depends on what you consider "properly intends" to mean.

I've taken this to mean that if you're inheriting from another class (B), that class is going to "properly intend to provide" any symbols (A) that are required to spell/declare, say, virtual method overrides (getA). (These can be forward declarations, though, and not necessarily full definitions.) I've mostly taken this approach because the alternative is tedious and provides no value.

Separately, I don't assume it means that the definition of class B provides a usable definition of class A, so I would still include "a.h" in "c.cc" (where A presumably is going to be used in the definition). I'd probably feel differently about this if we didn't use forward declarations so extensively.

I'd be curious to know what interpretation the IWYU tool uses by default (although this can be tuned with pragmas); jpgravel@ has been trying to get IWYU working in Chromium, and might have more context here. I suspect it might be more mechanical about this.

On Wed, Dec 14, 2022 at 3:19 PM Tao Bai <micha...@chromium.org> wrote:

This issue probably was already discussed, but I didn't find it.

Using an example to make the scenario clear

// a.h

class A {

}

// b.h 

#include "a.h"

class B {

  virtual A getA();

}

// c.h

#include "b.h"

#include "a.h" // Is this include necessary as A is a part of class B?

class C: public B {

  // Overridden B

  A getA();

}

// c.cc

#include "c.h"

#include "a.h" // Is this include necessary?

A C::getA() {}

IIUC, this case isn't what Include_What_You_Use tries to prevent as definition of class A is a part of the interface(class B) and the reference in c.cc is just for interface implementation, if A is used for other purpose as well, #include "a.h" is definitely required. 

If include a.h is necessary for above both cases, it seems .cc file should add most #include from its header file though there is no compilation penalty, but is kind of redundant to me.

Thanks

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAD48xCvPCvxfvR9h86vXYSYyBMhOO3cFSZ20BE4ibz-EwzOmFg%40mail.gmail.com.

[David Munro]

I won't claim it's the correct answer, but usually I just do whatever clangd --header-insertion=iwyu gives me (assuming it adds the right include, often I need to manually delete or correct headers but it's still a net time save for me).

In this case it has b.h includes a.h, c.h includes a.h and b.h, c.cc includes a.h

Cheers,
Dave.

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/CACwGi-4MHF6aBJM%2BBujp%2BuSn0FGmxfKs_W9mgxGPsbXq_qvydQ%40mail.gmail.com.