Skip to content

Commit

Permalink
Merge pull request CleverRaven#27683 from jbytheway/iwyu_prep
Browse files Browse the repository at this point in the history
IWYU support (part 2): Miscellaneous prep work
  • Loading branch information
kevingranade authored Jan 17, 2019
2 parents a67b4a0 + d27429b commit ed196fe
Show file tree
Hide file tree
Showing 42 changed files with 265 additions and 144 deletions.
4 changes: 4 additions & 0 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ The "???" is the issue number. This automatically closes the issue when the PR i

See https://help.github.com/articles/closing-issues-using-keywords/ for more.

## Tooling support

Various tools are available to help you keep your contributions conforming to the appropriate style. See [the relevant docs](../doc/DEVELOPER_TOOLING.md) for more details.

## Advanced Techniques

These guidelines aren't essential, but they can make keeping things in order much easier.
Expand Down
61 changes: 61 additions & 0 deletions doc/DEVELOPER_TOOLING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
## Astyle

Automatic formatting is performed by astyle. If you have make and astyle
installed then this can be done with 'make astyle'.

On Windows, there is an astyle extension for Visual Studio.

## include-what-you-use

[include-what-you-use](https://github.com/include-what-you-use/include-what-you-use)
(IWYU) is a project intended to optimise includes. It will calculate the
required headers and add and remove includes as appropriate.

Running on this codebase revealed some issues. You will need a version of IWYU
where the following PRs have been merged (which has not yet happened at time of
writing):

* https://github.com/include-what-you-use/include-what-you-use/pull/637
* https://github.com/include-what-you-use/include-what-you-use/pull/641

Once you have IWYU built, build the codebase using cmake, with
`CMAKE_EXPORT_COMPILE_COMMANDS=ON` on to create a compilation database
(Look for `compile_commands.json` in the build dir to see whether that worked).

Then run:

```
iwyu_tool.py -p $CMAKE_BUILD_DIR/compile_commands.json -- -Xiwyu --mapping_file=$PWD/tools/iwyu/cata.imp | fix_includes.py --nosafe_headers
```

We have to use IWYU pragmas in some situations. Some of the reasons are:

* IWYU has a concept of [associated
headers](https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-associated),
where each cpp file can have some number of such headers. The cpp file is
expected to define the things declared in those headers. In Cata, the
mapping between headers and cpp files is not nearly so simple, so there are
files with multiple associated headers, and files with none. Headers that
are not the associated header of any cpp file will not get their includes
updated, which could lead to broken builds, so ideally all headers would be
associated to some cpp file. You can use the following command to get a list
of headers which are not currently associated to any cpp file (requires GNU
sed):

```
diff <(ls src/*.h | sed 's!.*/!!') <(for i in src/*.cpp; do echo $i; sed -n '/^#include/{p; :loop n; p; /^$/q; b loop}' $i; done | grep 'e "' | grep -o '"[^"]*"' | sort -u | tr -d '"')
```

* Due to a [clang bug](https://bugs.llvm.org/show_bug.cgi?id=20666), uses in
template arguments to explicit instantiations are not counted, which leads to
some need for `IWYU pragma: keep`.

* Due to
[these](https://github.com/include-what-you-use/include-what-you-use/blob/4909f206b46809775e9b5381f852eda62cbf4bf7/iwyu.cc#L1617)
[missing](https://github.com/include-what-you-use/include-what-you-use/blob/4909f206b46809775e9b5381f852eda62cbf4bf7/iwyu.cc#L1629)
features of IWYU, it does not count uses in template arguments to return
types, which leads to other requirements for `IWYU pragma: keep`.

* IWYU seems to have particular trouble with types used in maps and
`cata::optional`. Have not looked into this in detail, but again worked
around it with pragmas.
3 changes: 2 additions & 1 deletion src/ballistics.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
#ifndef BALLISTICS_H
#define BALLISTICS_H

#include "dispersion.h"

class Creature;
class dispersion_sources;
class vehicle;
struct dealt_projectile_attack;
struct projectile;
Expand Down
9 changes: 2 additions & 7 deletions src/cata_utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,11 @@
#include <utility>
#include <vector>

#include "units.h"

class item;
class Creature;
struct tripoint;
namespace units
{
template<typename V, typename U>
class quantity;
class mass_in_gram_tag;
using mass = quantity<int, mass_in_gram_tag>;
}
class JsonIn;
class JsonOut;

Expand Down
24 changes: 14 additions & 10 deletions src/catalua.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include "game.h"
#include "item.h"
#include "item_factory.h"
#include "line.h"
#include "map.h"
#include "mapgen.h"
#include "mapgen_functions.h"
Expand All @@ -20,30 +19,35 @@
#include "player.h"
#include "pldata.h"
#include "requirements.h"
#include "rng.h"
#include "string_formatter.h"
#include "translations.h"
#include "weather_gen.h"

#ifdef LUA

#include "activity_type.h"
#include "bionics.h"
#include "field.h"
// Many pragmas here for headers which IWYU doesn't realise we need because
// they are used only inside src/lua/catabindings.cpp (the autogenerated file)

#include "activity_type.h" // IWYU pragma: keep
#include "bionics.h" // IWYU pragma: keep
#include "field.h" // IWYU pragma: keep
#include "filesystem.h"
#include "gun_mode.h"
#include "itype.h"
#include "line.h" // IWYU pragma: keep
#include "mapdata.h"
#include "mongroup.h"
#include "morale_types.h"
#include "morale_types.h" // IWYU pragma: keep
#include "mtype.h"
#include "mutation.h"
#include "npc.h"
#include "mutation.h" // IWYU pragma: keep
#include "npc.h" // IWYU pragma: keep
#include "optional.h"
#include "overmap.h"
#include "overmap_ui.h"
#include "overmap_ui.h" // IWYU pragma: keep
#include "requirements.h" // IWYU pragma: keep
#include "rng.h" // IWYU pragma: keep
#include "string_input_popup.h"
#include "trap.h"
#include "trap.h" // IWYU pragma: keep
#include "ui.h"

extern "C" {
Expand Down
10 changes: 1 addition & 9 deletions src/creature.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "pimpl.h"
#include "string_formatter.h"
#include "string_id.h"
#include "units.h"

enum game_message_type : int;
class nc_color;
Expand Down Expand Up @@ -48,15 +49,6 @@ struct mutation_branch;
using trait_id = string_id<mutation_branch>;
class ma_technique;
using matec_id = string_id<ma_technique>;
namespace units
{
template<typename V, typename U>
class quantity;
class mass_in_gram_tag;
using mass = quantity<int, mass_in_gram_tag>;
class volume_in_milliliter_tag;
using volume = quantity<int, volume_in_milliliter_tag>;
}

enum m_size : int {
MS_TINY = 0, // Squirrel
Expand Down
24 changes: 3 additions & 21 deletions src/io.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
* ...
* };
*
* The tag types are defined in io_tags.h so that this header need not be
* included to use them.
*
* The function must be declared with the archive type as template parameter, but it can be
* implemented outside of the header (e.g. in savegame_json.cpp). It will be instantiated with
* JsonArrayInputArchive and JsonArrayOutputArchive, or with JsonObjectInputArchive and
Expand Down Expand Up @@ -82,11 +85,6 @@
namespace io
{

class JsonArrayInputArchive;
class JsonArrayOutputArchive;
class JsonObjectInputArchive;
class JsonObjectOutputArchive;

/**
* Tag that indicates the default value for io should be the default value of the type
* itself, i.e. the result of default initialization: `T()`
Expand All @@ -110,22 +108,6 @@ struct empty_default_tag { };
*/
struct required_tag { };

/**
* Tag that indicates the class is stored in a JSON array.
*/
struct array_archive_tag {
using InputArchive = JsonArrayInputArchive;
using OutputArchive = JsonArrayOutputArchive;
};

/**
* Tag that indicates the class is stored in a JSON object.
*/
struct object_archive_tag {
using InputArchive = JsonObjectInputArchive;
using OutputArchive = JsonObjectOutputArchive;
};

/**
* The namespace contains classes that do write/read to/from JSON via either the Json classes in
* "json.h" or via the `io` function of the object to be written/read.
Expand Down
31 changes: 31 additions & 0 deletions src/io_tags.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#pragma once
#ifndef CATA_IO_TAGS_H
#define CATA_IO_TAGS_H

namespace io
{

class JsonArrayInputArchive;
class JsonArrayOutputArchive;
class JsonObjectInputArchive;
class JsonObjectOutputArchive;

/**
* Tag that indicates the class is stored in a JSON array.
*/
struct array_archive_tag {
using InputArchive = JsonArrayInputArchive;
using OutputArchive = JsonArrayOutputArchive;
};

/**
* Tag that indicates the class is stored in a JSON object.
*/
struct object_archive_tag {
using InputArchive = JsonObjectInputArchive;
using OutputArchive = JsonObjectOutputArchive;
};

}

#endif // CATA_IO_TAGS_H
16 changes: 2 additions & 14 deletions src/item.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
#include "cata_utility.h"
#include "debug.h"
#include "enums.h"
#include "io_tags.h"
#include "item_location.h"
#include "string_id.h"
#include "units.h"
#include "visitable.h"
#include "requirements.h"

Expand All @@ -30,15 +32,6 @@ class JsonOut;
class iteminfo_query;
template<typename T>
class ret_val;
namespace units
{
template<typename V, typename U>
class quantity;
class mass_in_gram_tag;
using mass = quantity<int, mass_in_gram_tag>;
class volume_in_milliliter_tag;
using volume = quantity<int, volume_in_milliliter_tag>;
} // namespace units
class gun_type_type;
class gunmod_location;
class game;
Expand Down Expand Up @@ -91,11 +84,6 @@ struct light_emission {
};
extern light_emission nolight;

namespace io
{
struct object_archive_tag;
}

/**
* Value and metadata for one property of an item
*
Expand Down
2 changes: 2 additions & 0 deletions src/item_location.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include <list>
#include <memory>

#include "map_selector.h"

struct tripoint;
class item;
class Character;
Expand Down
9 changes: 2 additions & 7 deletions src/item_stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,9 @@
#include <cstddef>
#include <list>

#include "units.h"

class item;
namespace units
{
template<typename V, typename U>
class quantity;
class volume_in_milliliter_tag;
using volume = quantity<int, volume_in_milliliter_tag>;
} // namespace units

// A wrapper class to bundle up the references needed for a caller to safely manipulate
// items and obtain information about items at a particular map x/y location.
Expand Down
1 change: 1 addition & 0 deletions src/json.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
*
* Further documentation can be found below.
*/

class JsonIn;
class JsonOut;
class JsonObject;
Expand Down
2 changes: 1 addition & 1 deletion src/lightmap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include <cmath>
#include <cstring>

#include "fragment_cloud.h"
#include "fragment_cloud.h" // IWYU pragma: keep
#include "game.h"
#include "map.h"
#include "map_iterator.h"
Expand Down
21 changes: 9 additions & 12 deletions src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
#include <iostream>
#include <locale>
#include <map>
#if (!(defined _WIN32 || defined WINDOWS))
#include <signal.h>
#endif
#include <stdexcept>
#ifdef LOCALIZE
#include <libintl.h>
#endif

#include "color.h"
#include "crash.h"
Expand All @@ -23,20 +30,10 @@
#include "output.h"
#include "path_info.h"
#include "rng.h"
#if (!(defined _WIN32 || defined WINDOWS))
#include <signal.h>
#endif
#include <stdexcept>
#ifdef LOCALIZE
#include <libintl.h>
#endif
#include "translations.h"

#ifdef TILES
# if defined(_MSC_VER) && defined(USE_VCPKG)
# include <SDL2/SDL_version.h>
# else
# include <SDL_version.h>
# endif
#include "sdl_wrappers.h"
#endif

#ifdef __ANDROID__
Expand Down
8 changes: 1 addition & 7 deletions src/map.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "lightmap.h"
#include "shadowcasting.h"
#include "string_id.h"
#include "units.h"

//TODO: include comments about how these variables work. Where are they used. Are they constant etc.
#define CAMPSIZE 1
Expand All @@ -33,13 +34,6 @@ namespace cata
template<typename T>
class optional;
} // namespace cata
namespace units
{
template<typename V, typename U>
class quantity;
class mass_in_gram_tag;
using mass = quantity<int, mass_in_gram_tag>;
} // namespace units
class emit;
using emit_id = string_id<emit>;
class vpart_position;
Expand Down
Loading

0 comments on commit ed196fe

Please sign in to comment.