Error: invalid use of incomplete type

VIVE L’AMOUR!
this is ROOT “v5-28-00-patches” on an Ubuntu 10.04.1 LTS / i686 with gcc 4.4.3.

In a piece of source code, I am using some “TGraph” objects, so I have:

#include “TGraph.h”

but, when I try to compile the code with ACLiC, I get:

(…) error: invalid use of incomplete type ‘struct TH1F’
/…/root/v5-28-00-patches/include/TGraph.h:46: error: forward declaration of ‘struct TH1F’

(…) error: invalid use of incomplete type ‘struct TAxis’
/…/root/v5-28-00-patches/include/TGraph.h:44: error: forward declaration of ‘struct TAxis’

These problems come because I do, for example:

MyGraph->GetHistogram()->GetMinimum()

MyGraph->GetXaxis()->SetLabelSize(0.06)

I don’t know why, but this problem was not visible in (much) older versions of ROOT (and/or with older gcc versions).

The solution is to add the following line into my source code:

#include “TH1F.h” // this will also automatically #include “TAxis.h”

Could you, please, put the following lines into the “TGraph.h” itself:

#ifndef ROOT_TH1F
#include “TH1F.h”
#endif

I am stupid. No?
Pepe Le Pew.

Hi Pepe-

This is common practice in C++ code to only forward declare classes when possible to improve compilation speed.  It is also an explicit part of ROOT's coding conventions:

root.cern.ch/drupal/content/c-co … ile_layout

The basic idea is that it makes sense to speed up compilation for almost everyone (who don’t use those access functions), and slightly ‘inconvenience’ people who need the headers to insert one line of code, as you do in this case.

As to why it wasn’t visible before, I would suppose that they swept through to remove headers when they were unnecessary.

Cheers,
Mike

VIVE L’AMOUR!
well, if compilation speed is really that much of a concern, let’s remove ‘#include “TAxis.h”’ from “TH1.h” and see what happens. :mrgreen:

I’m Pepe Le Pew, not Speedy Gonzales, anyhow. :laughing:

In my opinion, one should at least look at all the “Get*” methods of a particular class and “#include” all interfaces to all RETURNED data types (no need to “#include” interfaces to any “parameters” of methods, this should be handled by the user). To be really safe, a “#include” should appear for any data type RETURNED by any method (i.e. not only these named “Get*”) of a particular class.

A pitiful case, am I not?
Pepe Le Pew.

Well, then it wouldn’t compile at all, TH1 has TAxis class members. :slight_smile:

I understand your point, but you also have to look at it from a development standpoint. We are all developers, anyhow, if you’re compiling code. If you include unnecessary header files, this gets factored into that file’s dependency. Since it will be recompiled every time a change is made to any included header file, including the unnecessary ones, this can quickly increase the amount you’re recompiling without any reason. This can have real performance issues during compilation and have an adverse affect on development speed, especially for a mature package like ROOT.

VIVE L’AMOUR!
and how often do you change the layout of classes, like “TH1.h”?
Once you change it, most of files that use it will need to be recompiled anyhow.
Hence, I see no problem here.

The problem in ROOT is that one often needs to perform actions like:

SomeObject->GetSomething()->GetSomethingElse()->SetWhatever();

Now, I’m not interested in all these “intermediate” pointers at all.
Once I “#include” the proper interface for “SomeObject”, I’d like to have the whole “chain” working.
Well, it was working … though it was a long time ago, I admit. :smiley:

I am stupid. No?
Pepe Le Pew.

Hi Pepe,

We have been working on reducing the cost of any headers and cost ‘only’ was is needed. As a general rule, it is best for source code to explicitly include all the header file that are ‘used’ in the source file. This serves both as internal documentation and better dependency tracking.

Cheers,
Philippe.

[quote=“Pepe Le Pew”]
I am stupid. No?
Pepe Le Pew.[/quote]

According to Walter Bright, the creator of D programming language and the author of Digital Mars C/C++ compiler, work with header files is one of bottlenecks of modern C++ compilers. May be, you can wait a day while your code is compiling, but for most people long compilation time is annoying. I’m really happy how fast is ROOT’s compilations nowadays, and this is, I guess, because dependencies are minimized - many thanks to ROOT’s team for that!
I think, also, C++ is quite tough to parse: C also has headers, but compare how fast is the compilation of C code and how can be slow C++, it’s easy to notice even on stupid “Hallo world” program, which requires easy printf declaration for C and terrible ugly templates (from syntax/parser’s point of view) in case of C++.

So, the general rule for C++: minimize dependencies, include full declarations ONLY if you need them (to have a correct program).
Other modern programming languages have much more better module systems/separate compilation,
but this is already a different story.

P.S. I found Walter’s article, if you interested in C++ - you can read it :slight_smile: Of course, you do not need it if C++ is just an interface to ROOT for you :slight_smile:
drdobbs.com/blog/archives/20 … 4ATMY32JVN

P.P.S.
There is also a common “idiom”: you have a header file, which includes other headers you need, for example (from QT): //widgets, contexts, formats. etc. - all different headers in one.
<boost/mpi.hpp> - includes several boost.MPI headers, etc.

So, you can easily have such a headers, like ROOTTH.h with all TH staff, ROOTTree.h with different tree related classes and etc.