Open brettviren opened 3 months ago
Hi @brettviren , thanks for making this issue describing and tracking this problem.
Here I attach an example log file for demonstration purpose: log.txt The warnings in this log are specific to Boost 1.75.0 and Eigen 3.4.0 and with GCC 9.3.0 as Brett mentioned.
These are fixed with this commit: https://github.com/WireCell/wire-cell-toolkit/commit/f63acbf9a5f2a274663ac089cfad0898d536138a using the above #pragma
method described by Brett.
I fully agree that in the long run, we want to have gatekeepers to avoid too many sprinkled #pragma
. But I think we can do it later.
Related #122
For the "gatekeepers", could @brettviren suggest what is a proper granularity? Meaning should we have individual gatekeeper.h for each individual boost.h or put all the boost.h in one?
I see it as a balance between two extremes.
The first extreme is one gatekeeper per offending header and the other is one gigantic include-everything gatekeeper.
The extremes should be avoided but otherwise I feel broad optimum. A good rule of thumb is a gatekeeper should not include headers from more than one external package. In the case of Boost, I've tried to narrow to one Boost "topic", eg MultiArray.h
.
In the past I have looked at where headers from one external package are included to try to find patterns of use. Eg:
grep boost/iostreams **/*.h **/*.cxx
For that example, most already include Stream.h
or could do so without much harm. So, we might move the inclusion of the offending headers to Stream.h
and include Stream.h
in their place, if not already there.
Where no existing header conveniently serves as a gatekeeper, we can of course create a new one that has only that role.
Basic problem
We want to build WCT with strict compiler checks and so
-Wpendantic
and-Werror
are turned on by default.While we can WCT code itself "clean" this leads to problems when code that is
#include
is itself not "clean" and that will break building with-Wpedantic
and-Werror
.Problem likely to reoccur over time
This issue start to fix a specific instance of this class of problem. However, the problem can reappear as novel versions of existing externals are used to build WCT by the user. Feel free to re-open this issue when those are encountered.
Basic fix
To have our strict compilation while relaxing it for offending external code we must use a non-standard but effectively universal CPP macro
#pragma
.To apply the fix in general, follow this recipe:
#include
pagth.grep
or your favorite tool.#pragma
(see below)#include
of the offending headers in other source and#include
the "gatekeeper header".A "gatekeeper header" is a header file that includes some offending header(s) of some limited set and wraps them in suitable
#pragma
to turn off the strict compiling. This "gatekeeper header" is then used for#include
instead of including the "bare" offending headers.The initial problem
In this inaugural case the two offending headers show up when Boost 1.75.0 and Eigen 3.4.0 are used with GCC 9.3.0.
These are not directly included but are included by other's in Boost and Eigen, respectively.
WCT currently includes
boost/iostreams
related in many places and for different usages. So, it a new "gatekeeper header" for just these is warranted. Let's call thatWireCellUtil/Iostreams.h
in analogy to the existingMultiArray.h
. OTOH, the use of Eigen can likely always go through theArray.h
header and so we can instrument that as the "gatekeeper".Because the build turns on
-Werror
, anything that would merely be a warning becomes an error. This requires two#pragma
for each actual warning. Here is the guts ofMultiArray.h
showing things:The last chore is to replace
#include
of bare offending headers with inclusion of the gatekeeper. Again,grep
or your favorite search mechanism will help find the spots needing editing.