Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Header inclusion minimization #548

Closed
3 tasks done
TheJJ opened this issue May 3, 2016 · 9 comments
Closed
3 tasks done

Header inclusion minimization #548

TheJJ opened this issue May 3, 2016 · 9 comments
Labels
code quality Does not alter behavior, but beauty of our code lang: c++ Done in C++ code to-discuss Idea or suggestion that needs some discussion before implementation

Comments

@TheJJ
Copy link
Member

TheJJ commented May 3, 2016

In order to reduce compile times and prevent cyclic includes in the first place, all our files should be revisited:

  • In all header files, only non-pointer or non-reference data types are included from another header
  • All pointer or reference data types are forward declared in each header (class Bla;)
  • In the corresponding c++ file, those incomplete types are imported by #include "header"

Thus, complete types are only included in files where they are needed.
Template stuff and inheritance parents etc must be included in header files directly of course.

@TheJJ TheJJ added code quality Does not alter behavior, but beauty of our code good first issue Suitable for newcomers to-discuss Idea or suggestion that needs some discussion before implementation just do it You can start working on this, there should be nothing left to discuss labels May 3, 2016
@MaanooAk
Copy link
Contributor

This could help:

find . -iname *.h -exec cppclean "{}" \;  | grep "#include"

screenshot from 2017-01-11 01-14-10

To install cppclean

pip install cppclean

@VelorumS
Copy link
Contributor

@MaanooAk, some false positives if the traits are in separate headers.

@MaanooAk
Copy link
Contributor

@ChipmunkV I know, If you somehow pass all the files in one command, should be ok but it may crash... Not perfect in any way. I just used it recently in a much smaller project and thought could help if someone tries to actually to fix the headers...

@mic-e
Copy link
Member

mic-e commented Jan 19, 2017

We should definitely include cppclean as part of the code compliance checker. Does it produce any false positives?

@zuntrax
Copy link
Contributor

zuntrax commented Jan 21, 2017

First file checked was a false positive (not needing config.h in the assetmanager). Also crashes every few files. Might be nice for cleanups, but sadly not suitable for codecompliance.

@mic-e
Copy link
Member

mic-e commented Jan 21, 2017

Damnit... have i mentioned how the move to modules would solve everything™?

@zuntrax zuntrax added lang: c++ Done in C++ code and removed good first issue Suitable for newcomers just do it You can start working on this, there should be nothing left to discuss labels Jan 21, 2017
@zuntrax
Copy link
Contributor

zuntrax commented Jan 23, 2017

We had some discussion about this back in #263. The idea here was not forward declaring manually, but having declaration headers.

@zuntrax
Copy link
Contributor

zuntrax commented Jan 23, 2017

Also we should think about explicit instantiation declaration. http://en.cppreference.com/w/cpp/language/class_template

@simonsan simonsan added this to To do in code quality via automation Oct 4, 2019
@heinezen
Copy link
Member

heinezen commented Sep 8, 2023

The iwyu run in #1525 probably resolved most of this.

@heinezen heinezen closed this as completed Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Does not alter behavior, but beauty of our code lang: c++ Done in C++ code to-discuss Idea or suggestion that needs some discussion before implementation
Projects
code quality
  
To do
Development

No branches or pull requests

6 participants