As I’ve suggested multiple times in this context, the step that fixes this is #include juce_ComponentListener.h at the top of juce_ComponentListener.cpp.
That would ensure compiler breakage since juce_Component.h is required for juce_ComponentListener.h and not just juce_ComponentListener.cpp.
That’s also why I’m suggesting to remove all includes from the module headers and NOT keep juce_gui_basics.h as is, since it will hide some cases like the one you mention.
At the end juce_gui_basics.h would include only the headers that aren’t included by other public headers.
Also, will have to say that even if such transitive include can happen by mistake, it will be minor, easy to fix and opening the source file in VS code will show the problem immediately with a clear error message, compared to the current situation today where no types are recognized.
Maybe I’m missing something, but I don’t understand how that helps. By the time the compiler reaches #include "juce_ComponentListener.h", it’s already seen the full contents of juce_Component.h and juce_Component.cpp, so we still won’t get a warning in juce_ComponentListener.h that Component is undefined.
Step 1:
We remove the global include and comment out everything in the header.
//juce_gui_basics.h
#pragma once
//Really just everything commented out
//#include "juce_Component.h"
//#include "juce_ComponentListener.h"
//juce_gui_basics.cpp
#include "juce_Component.cpp"
#include "juce_ComponentListener.cpp"
After this step, creating a project that links against juce_gui_basics but isn’t including anything would fail to build. This is our ‘failing unit test’ in TDD-land.
Step 2, we’re adding includes until the code compiles.
//juce_Component.cpp
#include <juce_Component.h>
This doesn’t compile yet, because juce_ComponentListener.cpp still needs juce_ComponentListener.h. So we’re adding:
//juce_ComponentListener.h:
#pragma once
#include "juce_Component.h"
Now everything compiles. We could now just uncomment all those includes in juce_gui_basics.h and everything will work.
BUT! We want to do this right and safe! Se we’re adding: Step 3:
While keeping all headers commented out, start building JUCE demos/unit tests.
That will fail, because juce_ComponentListener.h isn’t included. So we add it back:
//juce_gui_basics.h
#pragma once
//Not needed anymore
//#include "juce_Component.h"
#include "juce_ComponentListener.h"
Adding juce_Component.h isn’t needed for this compilation, so we just remove it. Now if in the future we’re somehow super reckless and forget to include juce_Component.h from juce_ComponentListener.h, the transitive include through the .cpp files will not save us.
It is still possible in theory to get a transitive include by accident with this method, so you probably can find a few edge cases, but the scope of it would be pretty small, and easy to fix - the user who reports this will now say “I’m seeing red squiggles in juce_Component.cpp and they tell me that Component is missing”, which is a different report than “I’m seeing red squiggles on everything and can’t browse the code”.
The problem is that the code does compile before this final step. There’s nothing forcing you to add the #include "juce_Component.h", so you can still end up with lots of missing definitions.
@reuk It’s important to mention that transitive includes can happen in regular non-unity codebases too. But if x.cpp always includes x.h, and x.h usually has some includes in it as an automatic step the programmer always takes, they are very very rare.
I also find this thing interesting in your example:
#pragma once
//B.h:
struct B
{
A a;
};
I do have to say that if you ever programmed in C++ outside of JUCE this will probably look weird to you. Usually code needs some #include to work.
When I see something like that my mind immediately says something like “unless this only uses templates or primitive types, I’m gonna double check this file”. Because code like this should be the exception and not the rule in the code base.
I understand that you’re somehow looking for some 100% tool-enforced include verifier, but those don’t exist in C++.
In step 3 wouldn’t you end up adding “juce_Component.h” to “juce_gui_basics.h” not “juce_ComponentListener.h”, if there were multiple files that required “juce_Component.h” how would you know where to add it based on that step?
Also we haven’t discussed private source files for example imagine this
No. Because user code needs juce_ComponentListener.h to use ComponentListener.
However once I add it, juce_Component won’t be needed again in public headers.
I would add it only to each file that fails compilation.
For example I would add it to juce_Button.h, because unit tests using Button would fail.
Sure, those are the edge cases. But let’s explain what would happen in said edge case:
juce_Component.cpp is now missing SomeHelperClass.
juce_Component.cpp has all other includes it needs except for SomeHelperClass.h, so it shows red squiggles on exactly one class.
IDE shows a clear error message showing EXACTLY what type is missing, and in the some cases even automatically suggests you add it. This is the power you get when the IDE has a mostly-correct model of the code with only a few rare errors.
//juce_gui_basics.h
#pragma once
#include "juce_ComponentListener.h"
If you just include juce_Component.h the code won’t compile. That’s why it’s very important to test include changes with user code that uses the actual classes as a final step while limiting what’s exposed to the minimum.
I’m not sure why.
In my example unit tests that use Button, Label, Slider, etc would all fail to compile until you add the direct include (see above).
Tests for StringArray would fail unless it included String and Array properly. etc. Once your public header isn’t including the world, but only a couple of headers that aren’t included elsewhere, this solves most cases like these.
Sure but again - I’ve seen (non-unity) codebases that include a .cpp file once in a while. This leads to cases of transitive includes sometimes but the error message the IDE shows is super tiny and clear and only focused on the specific missing types and not the whole library showing false errors.
The solution to this problem is a static analysis tool like Include-What-You-Use, which detects types a file is using but is only importing transitively and not directly (the point being that every type you need should be brought in by a direct include).
My suggestion would be to follow Eyal’s described process, and once that is complete, then run IWYU over the entire JUCE codebase and keep adding any explicit includes you missed until it gives you no more warnings.
IWYU has built-in integration with CMake and is relatively easy to set up. But it currently won’t work at all, because the JUCE codebase isn’t structured in a typical way
Yes but now all the other headers you add to juce_gui_basics.h that come after juce_ComponentListener.h that also “should” require juce_Component.h in their header won’t need to include juce_Component.h because they get it as a transitive include via juce_ComponentListener.h, am I missing something?
I did try the technique exactly as described on a test project and as suspected it missed includes. You said it yourself that you only find it in the first one so something like Component which “should” be included in a lot of files won’t be without manual intervention.
I wonder if this process is improved by adding new includes to juce_gui_basics.h at the top so that they can’t be subject to transitive includes. Unfortunately you would still miss transitive includes in the cpp. Again though you could improve that by commenting all but the last cpp in the unity cpp file and comment them back in one by one in reverse order.
Personally I don’t see why we wouldn’t…
Turn off the Unity build file(s)
Start building all the individual cpp files
Resolve all the errors
At this point VS intellisense should be happy, there should be absolutely no missing includes whatsoever
Turn the individual files off, and enable the unity build files
Comment out all the includes in the unity header, and turn them back on one by one as needed, as per your suggested process
It’s also worth highlighting that the process ignores circular dependencies. Take something like Component and ComponentListener, if we follow the procedure blindly both should include the other in the headers! This is circular, at least one should use forward declarations (I suspect they could probably both be satisfied with forward declarations). If we limit the work to the source files only we should be able to avoid these kind of circular issues without adding unnecessary and noisy forward declarations in the headers too, particularly as I don’t see how they would help resolve the intellisense issue.
I would also champion include-what-you-use specifically as IIUC I believe it can help us to keep things clean by being able to keep on top of includes that can be removed! That being said it’s yet another thing to add to CI which needs some investigation.
We’re talking about situation where third header, say juce_Button.h is requiring juce_Component.h but isn’t requiring juce_ComponentListener.h?
I think this will usually solved before that stage if before adding the included back to the public header you will have every x.cpp file include x.h as the first include.
Granted, this is more of a solution by convention, not by tooling, but I believe that this is how most C++ code in the wild will do things to avoid transitive includes.
That sounds like a good idea to me!
Right, I was simplifying things to keep the discussion somewhat readable and not go into every nitty gritty aspect of the C++ code.
Usually in C++ we solve the problem of header dependencies either with direct includes or forward decelerations. Those choices are already made in JUCE modules. so when I say “include dependencies directly” this could mean juce_Component.h but could also be:
//Config.h
#pragma once
namespace juce
{
class Component;
}
//juce_ComponentListener.h
#pragma once
#include "Config.h"
IDEs and intellisense have no problem with forward declarations used in this way.
For sure, if this works with the JUCE codebase that would be awesome.
I do suspect that it would only work after many manual changes have been done like pull the preprocessor definitions from the module header into its own header file, but once it works it might be a way to enforce the good practice with tooling.
Having used VS2017 with different frameworks and then jumped to VS2022 with JUCE, I assumed the red squiggly lines were some dysfunctional trait of visual studio. It’s enlightening to see the problem is not with the IDE but with the framework (or at least - some combination of the two).
I appreciate @eyalamir’s clear and committed advocacy for the proposed solution as well as the detailed explanations. I hope I can have time to more carefully read this thread and absorb some of the wisdom here on both sides of the argument.
Just to emphasize from a joe-echmoe user: the red squiggly lines all over are extremely frustrating. They show up both in the editor and the error list. They appear, disappear, and reappear. Particularly as I am now trying to roll my own standalone plugin holder, digging more deeply into the framework code, it’s very painful to deal with all these false positives. I hope a solution is found soon.
I did find in the course of reading this/related threads that changing the last dropdown in the errors pane from “Build + IntelliSense” to “Build only” will clean the linker errors from the error list, which will be of great help when I’m focused on my code (wish there was something comparable for TODOs), but as I am trying to modify the standalone filter window I’m digging into the JUCE framework a lot and seeing many many more of these false positive errors.
@anthony-nicholls you seemed to have spent some time on this problem. What I still can’t quite understand, is how this problem isn’t coming up either more often, or not for you guys in the JUCE team.
This is only speculation on my end - but I suspect it does come up in the day to day work of the JUCE team (as they have to work with those cpp files often).
Since if you have Xcode/CLion/VS ReShaper you could get your setup to a point where those CPPs do look mostly correct (perhaps with slow auto complete/false positives once in a while), I think they’ve gotten used to living with those flaws, but would probably be a bit more reluctant to work on the parts of the code that may show that problem even with those IDEs.
In the past, I’ve worked on all kinds of projects that had similar configuration problems in some parts of the code, and the team’s productivity/enthusiasm was definitely much lower when it came down to new features/bug fixes in them compared to other parts/projects that did not suffer from it.
This is how juce_DocumentWindow looks like when I open it in last version of vs 2022. No squiggles.
Sometimes they do occur, mainly in h.files, as can be seen in the right hand file, one of my own. There the green squiggles falsly indicates “Function definition not found”, something I can live with.
I think it’s been better over time, a year ago or so they tended to be all over the place. I didn’t bother to much as long as Intellisense worked.
I’m not using the projucer, except for maybe at project start.
JuceHeader.h is included in each of my files and serves also as the precompiled header file, which I set up in the project settings of visual studio.
The build time for a project of some hundreds source files (except for the juce files) is less than 30s (debug mode) and if I make a change in one of those and rebuild it takes 1 or 2 seconds to rebuild.
Personally I’m pretty satisfied with the current build process and state of modules/include file handling. This of course doesn’t mean I don’t acknowledge the dissatisfaction other might feel.
Just wanted to share my view of things. Guess it’s a matter of ymmv…
