I stand by my assertion that it is not straightforward to add all of the necessary explicit includes. Doing this the naive way (removing includes of .h files from the top-level module .cpp, then adding them to inner .cpp files until the build works again) is not guaranteed to produce correct results, because includes in ‘earlier’ .cpp files will still be visible in ‘later’ .cpp files.
Moving all internal includes to a “config.h” file that gets included in every .cpp is not guaranteed fix this problem. If the config.h gets forgotten in any .cpp that appears later on in the top-level .cpp, the module will still build because of transitive includes through earlier .cpp files.
It also sounds like completion in VS Code may also currently be broken for JUCE headers, too (can anyone verify?). Therefore, we’d need to ensure that each internal header also included all the necessary dependencies, which again would be difficult due to transitive includes.
I think the only way to guarantee correctness would be to add a non-unity build mode for each module, and to test that mode in CI in combination with other commonly-used preprocessor options. This would take much longer than a couple of hours to put together, and would incur substantial ongoing maintenance costs.
In short, I’m not convinced that the benefits outweigh the costs. It also seems as though any benefits would mostly apply when working on first-party JUCE modules, so I’m interested to hear some concrete examples of problems that would be fixed by this change.
I understand your concerns about the ‘perfect’ way to do this with a tool that would 100% guarantee such a change would indeed include all needed .h files.
However, since this a change that even if done without 100% proofing of that would dramatically improve the day-to-day life of all JUCE devs, My suggestion is to make that change ‘naively’, and just incrementally fix the very rare undetected issues that would happen as a result.
I suggest that since the ‘worst case scenario’ here would be a few red squiggles that are left on a handful of .cpp files (and probably reported by the community and fixed fast), while the upside would be that most JUCE files would work just fine as-is.
Currently because of this problem, IDEs (even “high end” ones like CLion) sometime ‘screw up’ things like go-to definition. It’s also a big problem when browsing JUCE code. auto complete and other IDE refactoring tools are randomly breaking because of this, etc.
That is just due to VS Code naively trying to recursively parse headers. It may find juce_Component.h, but it might not be able to find juce_String.h Even though they’re all included in transitive includes of juce_gui_basics.h.
That is also what’s causing IDEs like Xcode/CLion/etc to spin up a lot more memory to find how to index functions like Component::getName() and eventually put them in the auto-complete box, as usually these IDEs cache the database of a header by parsing it and then parsing all #includes coming from it recursively.
Do the change ‘naively’ (just add the includes, remove #include module.h from module.cpp).
Test builds exactly the way they’re tested now. If this change would somehow cause a user reporting an error due to a combination of flags, that probably means a test for that set of flags needs to be added to CI anyway.
Make the change incrementally, starting with simple high level modules like `juce_product_unlocking’, ‘juce_osc’, ‘juce_dsp’ etc, splitting this into well encapsulated PRs so that over time that change would lead to a great benefit the JUCE community as a whole, many of which have reported the problems caused by this for years.
There were recent reports of Visual Studio and Intellisense being broken when parsing JUCE module code, but this has been fixed and is awaiting release (linked below). Perhaps this fix will also apply to VS Code.
Just for fun, here are other plenty of cases where similar problems were reported in all other IDEs:
It goes on…
Please @reuk. No other codebase in the world adds tests for a build system that isn’t supported just to add #includes for a few .cpp files.
Adding those is non-breaking, simple to do (can be done per module in an isolated way), can be iterative and will probably be a huge improvement to the lives of everyone here.
Let’s not make this more complicated than it is. Transitive includes is a common mistake that can happen in every C++ codebase out there (and it happened once in a while in my own code) and the fixes are trivial, similar to documentation errors or typos.
Please consider fixing this as this has been reported many, many times by users in all OS’s and all IDEs, and I bet the JUCE team experiences this on a daily basis.
I think it would be nice to have a small concrete example of the actual problem so we know what we’re does actually fix the issue. There could be a lot of work for very little reward if we don’t understand the actual issue. Also I assume this is only while browsing JUCE implementation code? If users aren’t employing unity builds and they only browse the headers then it’s not an issue right?
Not that many.
For example, most juce GUI objects just need to #include <juce_component.h>. They don’t need to explicitly include <juce_graphics.h> that is a requirement of it.
So what the compiler will have to parse is the “high level” include and it won’t need to even test for all the internal includes that are brought in as a part of this module.
As far as the compiler is concerned, there’s some std::map<std::string, bool> that tells the compiler to do nothing. You can benchmark this and see that even a million such lookups are probably in the time area of non-measurable difference
All of the above is related to a clean compilation.
In an incremental compilation that’s done after only some files have been changed, the compile process would be much faster since unless you’ve changed something in the #include dirty list, the compiler can decide to do nothing.
Currently, the current system is fast to compile as a whole but the compiler has to do a lot of work parsing the entire module for each change since it has no idea which headers from it can be completely ignored in the compile process.
I suspect that the speed change of incremental compilations in modules would be something the JUCE team (or anyone writing custom JUCE modules) would also see instantly.
Correct me if I’m wrong but I expect Component will need plenty of other things included too.
More than happy to be proven wrong here, but my understanding is that would only be true if we are actually compiling the cpp files, that’s different to satisfying a few IDEs, at the end of the day we would still be compiling one large cpp file so if anything changes the whole thing has to be rebuilt.
I would also be concerned but how certain preprocessor definitions and options are handled in headers you would effectively have t duplicate a bunch of this stuff in all the separate headers which runs its own risks. Obviously there are clean ways to break all this up but again that’s not a small amount of work it will take time.
If I’m honest I think there are some shortcuts/hacks where you can make the compiler treat each file in the unity build as if it is itself is the unity build. So it will still take time but if it’s just the unity build style that is the issue for some IDEs then I think it would achieve that goal.
Building with CMAKE_UNITY_BUILD=On is currently not possible without also excluding JUCE from the Unity Build.
Is using CMAKE_UNITY_BUILD=On any faster/slower than JUCE’s own Unity Build solution? Perhaps’s JUCE’s CMakeLists.txt could set this by default if it is any faster.
It would be nice to not have to use the JUCE module format and instead include only the headers you need. Sometimes you end up bringing in loads of bloat only to use 1 or 2 JUCE APIs. Top-level library includes is considered a bit of an anti-pattern these days is it not?
In theory JUCE should be faster as it should group closely related files together which overall reduces the amount of parsing required, as many of them will require the same headers.
Don’t forget we need to support the Projucer too.
I get there are benefits but only with costs too. If we switched JUCE away from unity builds we would suddenly have a lot of people complaining about the overall build times. Most users shouldn’t be changing JUCE source very often (ideally at all) so incremental builds shouldn’t be a big issue for most end users.
If you changed juce_somefile.cpp, a rebuild of that module would have to happen. That won’t change with my suggestion one way or the other.
However - if you modify juce_button.h and didn’t modifly juce_component.h, each module/unit would hit the #pragma once in juce_component.h, and stop there. There isn’t a need to reparse Component since the compiler knows for sure it hasn’t changed in all of the current contexts, which it doesn’t now.
That shouldn’t change.
To do that, you need to add another header file, config.h and have all headers in the module include it. That is a requirement and has to be done for code to not break.
That is also how it works in pretty much all C++ code BTW, this isn’t special to JUCE. You have one header of that module with all the commonly needed preprocessors.
No, you won’t have to duplicate anything. You will add a bunch of #include config.h, and that will have an include guard.
All these stuff is really how it always works if you open any C++ codebase in the world (except for JUCE).
No need for such hacks. I’ve already shown in the discussion linked above that this is trivial to do. Here’s an example for juce_osc:
(This is from 2 years ago, but I’m happy to send an up to date PR focusing on simple modules like that).
This change is really super super simple. You just add includes, add one config.h file for the common stuff, and remove the global module header include.
The only module where this would take some major work is juce_core since there’s a bunch of system stuff there that would be need to be moved into their headers, but for all other modules it is something that can be done by the end of the day for the entire codebase.