Skip to content

Commit

Permalink
Use "Include What You Use" on WarpX (#1947)
Browse files Browse the repository at this point in the history
* initial tests with IWYU

* added a couple of forward declarations

* used iwyu on more files

* progress

* used iwyu on more files

* progress with iwyu

* progress with iwyu

* fixed bug

* progress with iwyu

* progress with IWYU

* progress with IWYU

* fixed bug

* fixed bug

* progress with IWYU

* progress with IWYU + use forward declarations in WarpX.H

* first try with .def files

* fix bugs

* progress with IWYU

* progress with IWYU

* progress with iwyu

* correct copyright

* fixed bug

* fixed bugs

* fix missing include

* fixed bug

* fix bug

* fix bug introduced during last bugfix

* use iwyu on newly added files

* add space

* fix bug

* fix missing include

* fix missing include

* fix missing include

* fixed bugs

* fixed bug

* attempt at fixing issue with math functions

* added missing include

* fixed missing include

* using _fwd.H

* fixed bug

* progress with iwyu

* update AMReX branch

* enforce alphabetic order

* progress with iwyu

* use right version of amrex

* use right version of amrex for tests

* fixed bug

* fix another bug

* fix missing include

* fix missing include

* fix missing include

* updated amrex

* initial work to document new include strategy

* updated documentation

* Fix rst & private includes

* Remove accidentially added files

* Fix rst code blocks

* one more rst block

Co-authored-by: Axel Huebl <[email protected]>
  • Loading branch information
lucafedeli88 and ax3l authored Jun 25, 2021
1 parent 78ebf77 commit 0b6ddad
Show file tree
Hide file tree
Showing 279 changed files with 3,221 additions and 733 deletions.
71 changes: 65 additions & 6 deletions Docs/source/developers/repo_organization.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,74 @@ All WarpX header files need to be specified relative to the ``Source/`` director
- e.g. ``#include "Utils/WarpXConst.H"``
- files in the same directory as the including header-file can be included with ``#include "FileName.H"``

The `include order <https://github.com/ECP-WarpX/WarpX/pull/874#issuecomment-607038803>`_ and `proper quotation marks <https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html>`_ are:
By default, in a ``MyName.cpp`` source file we do not include headers already included in ``MyName.H``. Besides this exception, if a function or a class
is used in a source file, the header file containing its declaration must be included, unless the inclusion of a facade header is more appropriate. This is
sometimes the case for AMReX headers. For instance ``AMReX_GpuLaunch.H`` is a façade header for ``AMReX_GpuLaunchFunctsC.H`` and ``AMReX_GpuLaunchFunctsG.H``, which
contain respectively the CPU and the GPU implemetation of some methods, and which should not be included directly.
Whenever possible, forward declarations headers are included instead of the actual headers, in order to save compilation time (see dedicated section below). In WarpX forward
declaration headers have the suffix ``*_fwd.H``, while in AMReX they have the suffix ``*Fwd.H``.
The include order (see `PR #874 <https://github.com/ECP-WarpX/WarpX/pull/874#issuecomment-607038803>` _ and `PR #1947 <https://github.com/ECP-WarpX/WarpX/pull/1947>` ) _ and `proper quotation marks <https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html>`_ are:

1. In a ``<MyName>.cpp`` file: ``#include "<MyName>.H"`` (its header) then
In a ``MyName.cpp`` file:

1. ``#include "MyName.H"`` (its header) then
2. (further) WarpX header files ``#include "..."`` then
3. PICSAR and AMReX header files ``#include <...>`` then
4. other third party includes ``#include <...>`` then
5. standard library includes, e.g. ``#include <vector>``
3. WarpX forward declaration header files ``#include "..._fwd.H"``
4. AMReX header files ``#include <...>`` then
5. AMReX forward declaration header files ``#include <...Fwd.H>`` then
6. PICSAR header files ``#include <...>`` then
7. other third party includes ``#include <...>`` then
8. standard library includes, e.g. ``#include <vector>``

In a ``MyName.H`` file:

1. ``#include "MyName_fwd.H"`` (the corresponding forward declaration header, if it exists) then
2. WarpX header files ``#include "..."`` then
3. WarpX forward declaration header files ``#include "..._fwd.H"``
4. AMReX header files ``#include <...>`` then
5. AMReX forward declaration header files ``#include <...Fwd.H>`` then
6. PICSAR header files ``#include <...>`` then
7. other third party includes ``#include <...>`` then
8. standard library includes, e.g. ``#include <vector>``

Each of these groups of header files should ideally be sorted alphabetically, and a blank line should be placed between the groups.

For details why this is needed, please see `PR #874 <https://github.com/ECP-WarpX/WarpX/pull/874#issuecomment-607038803>`_, `PR #1947 <https://github.com/ECP-WarpX/WarpX/pull/1947>`_, the `LLVM guidelines <https://llvm.org/docs/CodingStandards.html#include-style>`_, and `include-what-you-use <https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md>`_.

Forward declaration headers
---------------------------
Forward declarations can be used when a header file needs only to know that a given class exists, without any further detail (e.g., when only a pointer to an instance of
that class is used). Forward declaration headers are a convenient way to organize forward declarations. If a forward declaration is needed for a given class ``MyClass``, declared in ``MyClass.H``,
the forward declaration should appear in a header file named ``MyClass_fwd.H``, placed in the same folder containing ``MyClass.H``.
Below we provide a simple example:

``MyClass_fwd.H``:

.. code-block:: cpp
class MyClass;
``MyClass.H``:

.. code-block:: cpp
#include "MyClass_fwd.H"
#include "someHeader.H"
class MyClass{/* stuff */};
``MyClass.cpp``:

.. code-block:: cpp
#include "MyClass.H"
class MyClass{/* stuff */};
Usage: in ``SimpleUsage.H``

.. code-block:: cpp
For details why this is needed, please see `PR #874 <https://github.com/ECP-WarpX/WarpX/pull/874#issuecomment-607038803>`_, the `LLVM guidelines <https://llvm.org/docs/CodingStandards.html#include-style>`_, and `include-what-you-use <https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md>`_.
#include "MyClass_fwd.H"
#include <memory>
WarpX-specific vocabulary
-------------------------
Expand Down
19 changes: 16 additions & 3 deletions Source/BoundaryConditions/PML.H
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,28 @@
#ifndef WARPX_PML_H_
#define WARPX_PML_H_

#include "Utils/WarpXProfilerWrapper.H"
#include "PML_fwd.H"

#ifdef WARPX_USE_PSATD
# include "FieldSolver/SpectralSolver/SpectralSolver.H"
#endif

#include <AMReX_MultiFab.H>
#include <AMReX_Geometry.H>
#include <AMReX_BoxArray.H>
#include <AMReX_Config.H>
#include <AMReX_FabArray.H>
#include <AMReX_FabFactory.H>
#include <AMReX_GpuContainers.H>
#include <AMReX_IntVect.H>
#include <AMReX_REAL.H>
#include <AMReX_Vector.H>

#include <array>
#include <AMReX_BaseFwd.H>

#include <array>
#include <memory>
#include <string>
#include <vector>

struct Sigma : amrex::Gpu::DeviceVector<amrex::Real>
{
Expand Down Expand Up @@ -170,6 +181,8 @@ public:

static void Exchange (amrex::MultiFab& pml, amrex::MultiFab& reg, const amrex::Geometry& geom, int do_pml_in_domain);

~PML () = default;

private:
bool m_ok;

Expand Down
36 changes: 29 additions & 7 deletions Source/BoundaryConditions/PML.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,45 @@
*
* License: BSD-3-Clause-LBNL
*/
#include "PML.H"

#include "BoundaryConditions/PML.H"
#include "BoundaryConditions/PMLComponent.H"
#include "Utils/WarpXConst.H"
#ifdef WARPX_USE_PSATD
# include "FieldSolver/SpectralSolver/SpectralFieldData.H"
#endif
#include "Utils/WarpXAlgorithmSelection.H"
#include "Utils/WarpXConst.H"
#include "Utils/WarpXProfilerWrapper.H"
#include "WarpX.H"

#include <AMReX.H>
#include <AMReX_Print.H>
#include <AMReX_Algorithm.H>
#include <AMReX_Array.H>
#include <AMReX_Array4.H>
#include <AMReX_BLassert.H>
#include <AMReX_Box.H>
#include <AMReX_BoxList.H>
#include <AMReX_DistributionMapping.H>
#include <AMReX_FArrayBox.H>
#include <AMReX_FBI.H>
#include <AMReX_FabArrayBase.H>
#include <AMReX_Geometry.H>
#include <AMReX_GpuControl.H>
#include <AMReX_GpuDevice.H>
#include <AMReX_GpuLaunch.H>
#include <AMReX_GpuQualifiers.H>
#include <AMReX_IndexType.H>
#include <AMReX_MFIter.H>
#include <AMReX_ParmParse.H>
#include <AMReX_RealVect.H>
#include <AMReX_SPACE.H>
#include <AMReX_VisMF.H>

#ifdef AMREX_USE_OMP
# include <omp.h>
#endif

#include <algorithm>
#include <cmath>
#include <memory>

#include <utility>

using namespace amrex;

Expand Down
1 change: 1 addition & 0 deletions Source/BoundaryConditions/PML_current.H
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#define PML_CURRENT_H_

#include "BoundaryConditions/PMLComponent.H"

#include <AMReX.H>
#include <AMReX_FArrayBox.H>

Expand Down
16 changes: 16 additions & 0 deletions Source/BoundaryConditions/PML_fwd.H
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/* Copyright 2021 Luca Fedeli
*
* This file is part of WarpX.
*
* License: BSD-3-Clause-LBNL
*/

struct Sigma;
struct SigmaBox;

class SigmaBoxFactory;
class MultiSigmaBox;

enum struct PatchType;

class PML;
27 changes: 21 additions & 6 deletions Source/BoundaryConditions/WarpXEvolvePML.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,33 @@
* License: BSD-3-Clause-LBNL
*/
#include "WarpX.H"
#include "Utils/WarpXConst.H"
#include "WarpX_PML_kernels.H"

#include "BoundaryConditions/PML.H"
#include "PML_current.H"
#include "Utils/WarpXProfilerWrapper.H"
#include "WarpX_PML_kernels.H"

#ifdef BL_USE_SENSEI_INSITU
# include <AMReX_AmrMeshInSituBridge.H>
#endif

#include <cmath>
#include <limits>

#include <AMReX_Array4.H>
#include <AMReX_Config.H>
#include <AMReX_Extension.H>
#include <AMReX_FabArray.H>
#include <AMReX_GpuControl.H>
#include <AMReX_GpuLaunch.H>
#include <AMReX_GpuQualifiers.H>
#include <AMReX_IndexType.H>
#include <AMReX_IntVect.H>
#include <AMReX_MFIter.H>
#include <AMReX_MultiFab.H>
#include <AMReX_REAL.H>
#include <AMReX_Vector.H>

#include <AMReX_BaseFwd.H>

#include <array>
#include <memory>

using namespace amrex;

Expand Down
9 changes: 7 additions & 2 deletions Source/BoundaryConditions/WarpXFieldBoundaries.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
#include "WarpX.H"

#include "WarpX_PEC.H"
#include <AMReX.H>

#include <AMReX_REAL.H>
#include <AMReX_Vector.H>
#include <AMReX_MultiFab.H>

#include <array>
#include <memory>

using namespace amrex::literals;

void WarpX::ApplyEfieldBoundary(const int lev, PatchType patch_type)
Expand Down
12 changes: 12 additions & 0 deletions Source/BoundaryConditions/WarpX_PEC.H
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,18 @@
#include "WarpX.H"
#include "Utils/WarpXAlgorithmSelection.H"

#include <AMReX_Array.H>
#include <AMReX_Array4.H>
#include <AMReX_Config.H>
#include <AMReX_Extension.H>
#include <AMReX_GpuQualifiers.H>
#include <AMReX_IntVect.H>
#include <AMReX_REAL.H>

#include <AMReX_BaseFwd.H>

#include <array>
#include <memory>

namespace PEC {
using namespace amrex;
Expand Down
14 changes: 11 additions & 3 deletions Source/BoundaryConditions/WarpX_PEC.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
#include "BoundaryConditions/WarpX_PEC.H"

#include "WarpX.H"
#include <AMReX.H>
#include <AMReX_Vector.H>

#include <AMReX_Box.H>
#include <AMReX_Geometry.H>
#include <AMReX_GpuControl.H>
#include <AMReX_GpuLaunch.H>
#include <AMReX_IndexType.H>
#include <AMReX_MFIter.H>
#include <AMReX_MultiFab.H>
using namespace amrex::literals;
#include <AMReX_SPACE.H>
#include <AMReX_Vector.H>

using namespace amrex::literals;

bool
PEC::isAnyBoundaryPEC() {
Expand Down
1 change: 1 addition & 0 deletions Source/BoundaryConditions/WarpX_PML_kernels.H
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#define WARPX_PML_KERNELS_H_

#include "BoundaryConditions/PMLComponent.H"

#include <AMReX.H>
#include <AMReX_FArrayBox.H>

Expand Down
9 changes: 7 additions & 2 deletions Source/Diagnostics/BTD_Plotfile_Header_Impl.H
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
#ifndef BTD_PLOTFILE_HEADER_IMPL_H
#define BTD_PLOTFILE_HEADER_IMPL_H

#include <string>
#include <AMReX_Array.H>
#include <AMReX_Box.H>
#include <AMReX_BoxArray.H>
#include <AMReX_Config.H>
#include <AMReX_REAL.H>
#include <AMReX_SPACE.H>
#include <AMReX_Vector.H>
#include <AMReX_MultiFab.H>

#include <string>

/**
* \brief Class to read, modify, and write plotfile header when
Expand Down
11 changes: 7 additions & 4 deletions Source/Diagnostics/BTD_Plotfile_Header_Impl.cpp
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
#include "BTD_Plotfile_Header_Impl.H"

#include "WarpX.H"

#include <AMReX_ParallelDescriptor.H>
#include <AMReX_PlotFileUtil.H>
#include <AMReX.H>
#include <AMReX_FileSystem.H>
#include <AMReX_INT.H>
#include <memory>
#include <AMReX_Print.H>
#include <AMReX_Utility.H>

using namespace amrex::literals;
#include <array>
#include <istream>

using namespace amrex::literals;

BTDPlotfileHeaderImpl::BTDPlotfileHeaderImpl (std::string const & Headerfile_path)
: m_Header_path(Headerfile_path)
Expand Down
14 changes: 14 additions & 0 deletions Source/Diagnostics/BTDiagnostics.H
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@
#define WARPX_BTDIAGNOSTICS_H_

#include "Diagnostics.H"
#include "Diagnostics/ComputeDiagFunctors/ComputeDiagFunctor.H"
#include "Utils/WarpXConst.H"

#include <AMReX_Box.H>
#include <AMReX_Geometry.H>
#include <AMReX_IntVect.H>
#include <AMReX_MultiFab.H>
#include <AMReX_REAL.H>
#include <AMReX_RealBox.H>
#include <AMReX_Vector.H>

#include <limits>
#include <memory>
#include <string>

class
BTDiagnostics final : public Diagnostics
Expand Down
Loading

0 comments on commit 0b6ddad

Please sign in to comment.