On Tuesday 24 November 2009, Zach Welch wrote: > On Tue, 2009-11-24 at 16:54 -0800, David Brownell wrote: > > On Tuesday 24 November 2009, [email protected] wrote: > > > It is an error to require other headers to be included first, so declare > > > what we can (struct declarations) and include the rest (Jim types). > > > > This one seemed out of step with the rest of these 80+ patches. > > > > Any particular reason why that should be an "error"? > > > > Well.... I suppose that might be dropped, but that patch came out of > writing the testee target driver that comes next.
Like I said: out of step. :) > That code is > ridiculously minimal, in that it doesn't even include target.h yet. If > that got included in the C file first, it might solve the problems that > this patch addressed. > > That said, it feels like very bad form to omit forward declarations or > bare minimum #include directives that prevent a header from compiling by > itself. Yes and no ... that's a simple rule (and thus attractive), but going that way does have downsides where including one file then pulls in mountains of stuff that's often not needed. And other stuff which may have been included already... That said, we *now* have a mess, and sorting it out isn't very straightforward. I looked at a patch like this not long ago, and decided against it. One reason is that #including "jim.h" pulls in a huge ball of yarn that is not actually necessary for all code accessing target_type. In short: there's a second principle this fights against: not needing to include headers for substantial features unless you're actually going to use those features. What I'd rather see happen is that headers get dis-entangled from each other, not get more tightly coupled. And this particular little mess seemed to deserve more than a quick fix. > If you feel that this header should always be #included _after_ > target.h, then I'll rewrite the patch for that, but I hate the idea. Since you're sort-of-asking: I think it shouldn't be included at all in most cases. And if it's got to be include in some order ... I don't object to such requirements, they're pretty routine. > Personally, I think this is another style point that has no objective > winner, so what _exactly_ do you want in regards to headers? And don't > tell me, write it up for the Style Guide. :) I asked a question ... I wasn't asserting any kind of style point! Though it seemed to me that you were ... hence my question. > Now, I know you did some cleaning in this particular area, which my > recent purge of the struct typedefs allowed. Prior to that effort, > forward declarations like those I added in this patch weren't even > possible, so that allows 90% of the stuff to be decoupled and solves > most of the problems. It's still much better than being forced to > include all of the headers that included the type definitions, though > Jim remains a notable exception to that rule. Yeah, Jim being a mess. So until it gets a better fix, I'm kind of inclined to leave things as they are ... rather than make changes that -- by at least the "entanglement" metric -- make some things worse. > In summary, I think headers that don't compile by themselves (except for > the system headers provided through config.h) are broken, If it's OK for system headers, it should be OK for other headers. There's nothing magic about system stuff. The evolution which produced system headers hit all these same issues ... and decided that it's appropriate to have headers require other headers to have been included first, in various cases. Those requirements are even documented. > but there is > much work to be done with the cleaning up the headers further in > general. I'll get back to that again at some point, since removing the > struct typedefs has opened new opportunities for refactoring. I'm all for that cleanup. Stuff associated with "jim.h" is one obvious rats-nest to clean up. There are others too. It'll take quite a while to sort it all out cleanly though... - Dave _______________________________________________ Openocd-development mailing list [email protected] https://lists.berlios.de/mailman/listinfo/openocd-development
