Includes of SHELLmain are including like ALL the STL headers

Reporter: JoshDreamland  |  Status: open  |  Last Modified: October 06, 2017, 06:13:57 PM

A million years ago, for reasons of parse efficiency, I told people not to include STL containers (other than string) from SHELLmain. Fast forward to 2017, where I cannot name an STL container or adapter that is not included from SHELLmain.

This is tickling all sorts of issues with running JDI on GNUC++17 code.

Facing facts, JDI needs to be updated or replaced, moving forward. I am in favor of replacing it, on the grounds that, while Clang has not gotten any smaller over the years, the internet, users' bandwidth, and users' expected development platform size have gotten much, much bigger. A 300MB libclang dependency is no longer anything to be grumpy about (and we already had a 300MB compiler dependency on Windows, anyway).

In the meantime, JDI can't handle the weird-ass variadic templates and hoo-hah going on these new headers (it's largely #ifdef'd out, but additions to GNUC are probably screwing us, as well).

That being the case, we need to stop including these containers from the main source file, until JDI is solved. This will allow the string workaround to work more effectively, which is biting a lot of people, lately.

I suspect that this is, indirectly, what's causing #1006 (at very least for TKG).

I am filing a bug for this because literally anyone can go through and remove references to STL containers (eg, std::map) from the main headers (moving them into internal headers used only by actual implementations). This isn't something I feel like doing, so I'm not going to jump to it. I'd probably sooner solve JDI.

RobertBColton  
Ok, so it turns out some of our issues had nothing to do with templates like we thought. Me and @fundies spent all night debugging JDI to find the true source of the crashes originally reported in #944.

I finally came up with having him add the following to the top of enigma-dev/CompilerSource/JDI/src/Parser/readers/read_qualified_definition.cpp:

cout << (const char*)token.file << ' ' << token.linenum << '\n';

This told us the following right before the crash:

/usr/lib/gcc/x86_64-pc-linux-gnu/7.2.0/../../../../include/c++/7.2.0//type_traits 2084

Investigating that file we discovered this:

using __strictest = __strictest_alignment<_Types...>;

Eventually we were able to reproduce it with simply using meh = int; at the top of SHELLmain.cpp where JDI then crashes instead. So it looks like JDI actually just can't handle C++11's using alias syntax.

For the record the crash is on this line:


JoshDreamland  

The excess STL headers are no longer directly hurting anything, but their existence remains pointless: the headers including them aren't even using them. Just the sources including those headers are. But it's not dangerous, anymore—just sloppy.

I'm afraid to ask anyone to remove them, because there are extra steps to making sure they aren't necessary. The source files need to include them, but those sources should include their corresponding header, first, to ensure that all headers are valid on their own. I also have a feeling that some of the headers are, eg, declaring maps in namespace enigma.

fundies  

So how would you want to handle headers that declare std::maps as members of classes in headers? Things like:

#include "map"

class sounds
{
public:
std::map<int, data>
}

JoshDreamland  

It's okay for headers to do that, as long as they're not included from SHELLmain.cpp; we have a lot of internal headers like that (especially for SDK-specific definitions).
fundies  

there are many things in enigma namespace that do this and declare externs that are included from shellmain
fundies  

#1096 marked all stl shit
Please sign in to post comments, or you can view this issue on GitHub.