CppCoreGuidelines icon indicating copy to clipboard operation
CppCoreGuidelines copied to clipboard

SF.5 "file must include its own interface" can partly check for SF.11 "headers selfcontained.

Open DdOtzen opened this issue 3 months ago • 4 comments

I always ensure that the header defining the interface is included as the first one, as this automatically tests for SF.11, ensuring that the header file is self-contained.

I have read issue #366, were a similar rule is rejected with:

Components are not always neatly divided into A.h/A.cxx one-to-one correspondence. Furthermore, if A.h contains the interface of component A, then it should be self-contained.

But it is a long time ago (10 years). And I am NOT suggesting a rule that require the "name" (name as in <name>.<ext>) to be the same, for the .h and .cpp file.

What I suggest is that rule SF.5 is updated so that is requires or at least recommends that the header file that defines the interface is put first in the .cpp file, with the reason that it implicitly tests for rule SF.11. Especially in cases where there is only one .h file.

Additionally I suggest a rule [SF.?23?] that every .h file in the project is included as the first file in at least 1 (one) .cpp file.

Whether this rule belongs as an enforcement or a note to rule SF.11, or as a rule in itself I have no strong opinion about.

Applying the rule of "my own interface first" means that in all the cases (in my experience the major part) where there is only one .h file defining the interface, Rule SF.5 is fulfilled, Rule SF.11 is tested, and rule SF.?23? is fulfilled for most files in the project, so only the cases with multipli interface files, or other .h files need a dedicated test .cpp that just include that file to test for rule SF.11.

DdOtzen avatar Oct 13 '25 10:10 DdOtzen

I suggest a rule [SF.?23?] that every .h file in the project is included as the first file in at least 1 (one) .cpp file.

As-is, that would flag a lot of reasonable header-only code.

But as far as including nterface header first I wouldn't mind seeing a note in SF.5 acknowledging #366 as at least somewhat common practice.

See also discussion in https://github.com/isocpp/CppCoreGuidelines/issues/981 where some include ordering ideas were tossed around, including, again, interface-first, but without a clear core guideline emerging.

cubbimew avatar Oct 16 '25 18:10 cubbimew

As-is, that would flag a lot of reasonable header-only code.

Yes, that's the idea. An effective way to check that all headers can be included standalone, is to include them standalone.

So a header-only file would have a test file in the shape of a .c/.cpp file with just one line, being the #include of the header-only file.

example; my_header_only.h:

#define something some thing
typedef int my_int;

my_header_only_inc_test.cpp

#include "my_header_only.h"

When my_header_only_inc_test.cpp is compiled no code is generated but the syntax is checked. I.e. if the header contained typedef my_float my_other_float;, the compiler complaints that my_float is not defined.

One could set a rule to make such a include test file for all headers, but I think it is simpler to only do this for the header-only files.

At the same time I acknowledge that such a rule may be out of scope for this CppCoreGuidelines, but the arguments for using this pattern remains the same :-)

DdOtzen avatar Oct 30 '25 07:10 DdOtzen

See also discussion in https://github.com/isocpp/CppCoreGuidelines/issues/981 where some include ordering ideas were tossed around

in #981 there are some discussion about how "interface first" can create an issue if that interface.h contains defines directly or indirectly that affects other headers included after that one. What the discussion leaves out, is the scope of impact! And what do I mean by that? Well lets take the example from #981

// interface.h
#include <string>
#include <boost/algorithm/algorithm.hpp>
#include "../include/my_header1.hpp"
/* My interface code */

// impl.cpp
#include "interface.h"
#include <iostream>
#include <boost/any.hpp>
#include "../include/my_header2.hpp"
/* My impl code */

And we consider the impact in workload for bug hunting and fixing in these two cases.

  1. We stick to the interface first rule, and let is break the impl.cpp
  2. We design the includes in the impl.cpp to avid the issue, and hope all the other 100 users of that header do the same.

in case 1, I have to fix that header/interface while working on the implementation of the interface. In case 2, an unknown number of people have to fix it at an unknown point in time, an unknown amount of times.

DdOtzen avatar Oct 30 '25 07:10 DdOtzen

As-is, that would flag a lot of reasonable header-only code.

Yes, that's the idea. An effective way to check that all headers can be included standalone, is to include them standalone.

So a header-only file would have a test file in the shape of a .c/.cpp file with just one line, being the #include of the header-only file.

example; my_header_only.h:

#define something some thing typedef int my_int; my_header_only_inc_test.cpp

#include "my_header_only.h" When my_header_only_inc_test.cpp is compiled no code is generated but the syntax is checked. I.e. if the header contained typedef my_float my_other_float;, the compiler complaints that my_float is not defined.

One could set a rule to make such a include test file for all headers, but I think it is simpler to only do this for the header-only files.

At the same time I acknowledge that such a rule may be out of scope for this CppCoreGuidelines, but the arguments for using this pattern remains the same :-)

While I agree that headers should generally include anything that defines things they use so you don't need another header, that also can create two issues:

  1. Using header's like Microsoft's windows.h that includes everything under the sun.
  2. "Headers" that are not really headers in the normal sense, but shared stuff that gets included without header guards into files.

While I'm sure there are some other use cases, I'm only going to focus on these two.

In the first case, this causes compile times to get massive since its not the minimal set of headers that are needed for compilation. This is often the cause of long compile times. This would allow code to pass your test, but is itself a really bad practice.

In the second case, this can effect a lot of especially legacy code that may or may not be well understood for why it does it. I'm dealing with a case on a project now where we have a number of "header" files that don't have include guards because they're meant to be dropped into some specific places but the original authors wanted the information shared - the team hasn't had the time to dig in and see if there is a better pattern to be used as there are a lot of bigger concerns being dealt with. But you'll likely find this in especially older projects.

So while this might be a nice thing to have, it can also cause a ton of problems. At best it would have to be a recommendation with very lax enforcement by default due to potential fallout with a lot of code that is out there.

BenjamenMeyer avatar Oct 31 '25 13:10 BenjamenMeyer