The International Simutrans Forum

 

Author Topic: Cleanup of main includes  (Read 227 times)

0 Members and 1 Guest are viewing this topic.

Offline ceeac

  • Devotee
  • *
  • Posts: 135
Cleanup of main includes
« on: June 15, 2020, 01:53:04 PM »
This patch moves all "main" includes (e.g. "foo.h" for "foo.cc") to the top of the .cc file such that the main include is the first one in the .cc file. This makes sure that all headers include their necessary dependencies.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9992
  • Languages: De,EN,JP
Re: Cleanup of main includes
« Reply #1 on: June 15, 2020, 02:12:30 PM »
Did you test this on more than one compiler? Because in some cases having the include first did not compile (at least five years ago) due to some circular references. That why many class xyz; dummy declarations are there, which does not work if a real class is needed in the header.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5642
  • Languages: EN, NO
Re: Cleanup of main includes
« Reply #2 on: June 15, 2020, 07:57:18 PM »
Unless there is missing an "also" in the seconds sentence, or something else is missing entirely, the original post doesn't make any sense.

As for the second post, circular reference error shouldn't be compiler dependent.

Personally, I like to have inclusion of system headers (#include <>) before the project's own headers (#include ""). And the implementation (the cc file) may require more system headers than the interface (the h file). I'm not sure what has been typical, if anything, in Simutrans. (I'm too exhausted by the heat at the moment to check.)

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9992
  • Languages: De,EN,JP
Re: Cleanup of main includes
« Reply #3 on: June 16, 2020, 12:13:15 AM »
Usually it is first the system includes, then the dependencies and finally the own include. But this is probably just my own preference too.

Offline ceeac

  • Devotee
  • *
  • Posts: 135
Re: Cleanup of main includes
« Reply #4 on: June 16, 2020, 07:50:47 PM »
Did you test this on more than one compiler?
Yes - This compiles fine on Linux (clang/gcc), WIndows and macOS; I have not tested with mingw . Just moving the incluses to the top results in compiler errors, but I fixed those.

Personally, I like to have inclusion of system headers (#include <>) before the project's own headers (#include "").
IME having system includes after project-specific includes is better. Suppose you have a.h/cc and b.h/.cc where b.h contains a call to memcpy but does not #include <cstring>. This does not result in a compile error if you #include <cstring> in both a.cc and b.cc. If you put the main include first, this results in a compile error (unless the cstring header is included via some other header file). This also completely eliminates weird stuff like silently different compilation based on whether another header file is included before or not (for example windows.h which #defines min/max).

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9992
  • Languages: De,EN,JP
Re: Cleanup of main includes
« Reply #5 on: June 16, 2020, 11:42:29 PM »
But this defies the possibility to shadow system/library functions. If I define "new" (or malloc, like for debug code) in the include and then include the standard header afterwards, which will be used? I assume it will be the standard one ...

Also with windows, the include order is critical, since some Windows functions are exclude/included when the order is wrong.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5642
  • Languages: EN, NO
Re: Cleanup of main includes
« Reply #6 on: June 17, 2020, 06:12:32 PM »
This also completely eliminates weird stuff like silently different compilation based on whether another header file is included before or not (for example windows.h which #defines min/max).
As prissi writes, sometimes the other way around just doesn't work. Fortunately, Simutrans only includes windows.h very few places. And those places perhaps include very little else.

Suppose you have a.h/cc and b.h/.cc where b.h contains a call to memcpy but does not #include <cstring>. This does not result in a compile error if you #include <cstring> in both a.cc and b.cc.
And why is this a problem? Sure, you get a compilation error once you include b.h in c.cc that does not include cstring, but that leads just to the compilation error you wanted in the first place. It is just offset in time, and no harder to fix. And it doesn't fully solve the problem early either, because if a.h includes b.h, then maybe a.h is missing the inclusion of cstring. It just works until suddenly b.h doesn't need cstring anymore.