Re: [boost] [Stacktrace] Second review begins today 17th Mar ends 26th Mar

On 3/19/2017 5:34 AM, Niall Douglas via Boost wrote:

On 19/03/2017 06:38, Antony Polukhin via Boost wrote:

...

2017-03-18 17:39 GMT+03:00 Peter Dimov via Boost :

...

Antony Polukhin wrote:

...

windows.h is a minor problem of a single platform.

windows.h is not a minor problem at all. It defines one million macros.

dbgeng.h defines 10M macros. Getting rid of windows.h solves only small part of the problem.

Looks like the right solution would be not to minify headers that use the windbh.h/dbgeng.h but rather write a correct forward declarations of windbh.h/dbgeng.h stuff, just like Boost.WinAPI does.

My COM knowledge is limited, so I'd *really appreciate* some help with dbgeng.h. This will also fix multiple MinGW issues and will allow a better MinGW (and even Clang on Win) stacktracing out of the box.

I'll take care of windows.h stuff soon.

Putting on my review manager hat, here is my position on :

1. The size of windows.h is Microsoft's fault, not the code which includes it.

2. *Gratuitous* includes of windows.h is a showstopper for a high quality library, so including windows.h just for one or two APIs is a bad thing.

3. If however a library really makes extensive use of windows.h and if the user prefers the header only edition of the library, then including all of windows.h is *the price they pay*. They have the choice: either go header only and include windows.h, or don't go header only and don't include it.

If a library makes extensive use of windows.h in a header file which only includes functionality which needs windows.h, then naturally an end-user including that header file gets windows.h included. But if I am using some libraries header file, which has a number of constructs I would like to use, none of which needs windows.h, I do not want windows.h included. I realize that this is sometimes difficult for a library developer to do, but it should be a relatively easy task compared to the high level C++ development that most Boost library developers actually do. Is splitting header files into particular functionality really that hard ? if the library developer offers specific header files for windows.h functionality and functionality not using windows.h, while still providing a general header file which includes both, then I as an end-user can at least choose whether windows.h is going to be included or not depending on what header file I include. BTW substitute "any-ridiculously-large-header-file-which-includes-endless-nondistinct-macros-and-constructs" for windows.h and the argument above is exactly the same. In other words I am in 100% agreement with Peter even while I realize that separating windows.h functionality in stacktrace may be some work for Antony. But given that the windows.h behemoth is Microsoft's fault the general principle of C++, of not "using" what one does not need, still applies.

4. In case anyone thinks I'm being unreasonably lax here, nobody much complains about header only inclusion of etc. That's because the size of is *Microsoft's fault*. It's the cost they impose on every Windows dev. We library developers are not at fault. Blame Microsoft.

5. I think it's highly unreasonable to ask Antony to remove windows.h from the headers only build. The Windows backend unavoidably uses COM with all that magic compiler-specific GUID attribute markup and such fun, and the headers brought in are auto generated from COM IDL and hand tuned by Microsoft. Hacking that out just to tick a box for some people will produce a much more brittle, unreliable and hard to maintain library.

Peter did not suggest not using windows.h itself AFAICS. He just suggested that stacktrace functionality which does not need it be segregated into specific headers which do not include it. Furthermore if stacktrace still has the general header, and promotes the use of it, rather than specific headers, that's fine as long as the end-user has a choice. OTOH if stacktrace functionality which appears not to need the constructs in windows.h still does need it under the covers in the header only version of the library, then I understand Antony's objection.

I as review manager certainly will not stop Stacktrace entering Boost on this point. If anything, if Antony ruins the library just to eliminate inclusion, I'd actually recommend its rejection. I appreciate that this will not be a popular opinion, but I'm the review manager and that's my judgement on it.

Niall