Godot: Coding standards: Headers

[akien-mga]

As a monolithic issue (#3916) does not make it easy to discuss specific points, let's split some topics in their own issue (always referencing the main one) so that we can progress forwards.

---

Here I'd like to discuss the inclusion of headers in .cpp and .h files.
I can see two main points:

Ordering of headers

Currently I can't see a specific ordering scheme for headers in cpp and h files, see e.g. https://github.com/godotengine/godot/blob/master/drivers/unix/memory_pool_static_malloc.cpp and https://github.com/godotengine/godot/blob/master/drivers/unix/memory_pool_static_malloc.h

I would propose this format (taking the above drivers/unix/memory_pool_static_malloc.cpp as example):

/* memory_pool_static_malloc.cpp */
/* License header */
                                                // newline
// own header file comes first, only in .cpp of course
#include "memory_pool_static_malloc.h"
                                                // newline
// engine specific headers come then, in alphabetical order
#include "error_macros.h"
#include "os/copymem.h"
#include "os/memory.h"
#include "os/os.h"
                                                // newline
// system-wide and/or thirdparty headers come then, with `<>` format
// (proper include path should be given for thirdparty headers so that both
// thirdparty and system can coexist without having to use dirty defines everywhere)
// we write the .h part too because we're not Windows Explorer ;)
// also alphabetically sorted
#include <stdio.h>
#include <stdlib.h>
                                                // newline
/* code starts here */

Inclusion of headers used in the cpp file: cpp or corresponding h file?

As mentioned in https://github.com/godotengine/godot/pull/6920#pullrequestreview-5836867, we need a convention regarding where to place headers when they are used only in the cpp file.

Here I have little experience so I would welcome input from veteran C++ developers.

Intuitively, I would propose to put the includes in the files that uses it, i.e. when an external header is needed only in the .cpp file and not the .h one, put the include in the former. The .h therefore would only contain the includes necessary for its classes and methods signatures.

WDYT?

[volzhs]

I like it. It will prevent to include same header file also.

[akien-mga]

Some more opinions, or should we consider it agreed upon? :)

[RandomShaper]

That header ordering is advisable and widespread.

Also I find a good thing keeping the least number of inclusions a move them to the .cpp files wherever possible.

Also forward declarations to save some more inclusions could be part of the Godot coding conventions.

[akien-mga]

Also forward declarations to save some more inclusions could be part of the Godot coding conventions.

Could you give an example for a C++ newbie like myself? I googled a bit a while ago but did not fully understand what forward declarations refer to in this context.

[RandomShaper]

I'm glad you ask! :)

It's something already used extensively in the Godot's source code. So I'll take an example from there:

Array makes use of Variant but as long as core/array.h just include references and/or pointers to that type, there you can just go ahead with class Variant;. That's because the compiler doesn't need the definition of the Variant class for pointers/references; letting it know that class exists is enough.

In core/array.cpp the Variant class itself is used to the compiler must know its definition and the core/variant.h header must be included. That theoretically save the compiler from parsing a header file (core/variant.h) on .cpp files including it as long as they don't need the Variant's class body as well.

Well, Variant is not the best example as it's probably included everywhere anyway, but I think it suffices just to show how forward references work.

[akien-mga]

Good, I think we have an agreement on this one. If anyone has an idea for some script magic that would implement the convention without us having to go through all files manually, that would be cool :D

[ghost]

First of all thank you for your report and sorry for the...

Hey! Guess what 2-year-ago-@akien-mga, you did it! eb892cd

[akien-mga]

Hehe, well we still lack a way to enforce a proper ordering of #includes though as described in the OP. But that should go in the documentation I guess, I'm not sure how to get this enforced with static checks.