On Wed, 2009-11-25 at 13:08 -0800, David Brownell wrote: > 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: [snip] > 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.
But we can't use any features in that header without the patch I made. The header does not compile. jim.h was the only header I #included. The rest were all struct forward declarations for types used in the header's API, without which I got warnings building testee.c. Presently, you have to include all of those headers in the source file, which is exactly the opposite of what we want. Right? > 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. Except for jim.h, it would have been the correct long-term fix, and that last header can be treated as sacrosanct (i.e. all but a system header). I am not disagreeing with your principles, only your interpretation of them in context of the actual patch I had supplied. > > 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. Routinely broken, by design you say. Fascinating. ;) > > 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. If you knew to question my own assertion, then you have an opinion that constitutes your own assertion on style. Or is this logic fallacious? > > 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. But that's just it, this problem is new. I once created a test suite that checked the headers for this exact problem, when I did the first wave of simplifying the header madness. In all likelyhood, this appears like a regression in the ability to compile OpenOCD's header files. Anyway, it's still better than when I got here; I think you got on the ride after I shut down the circular dependencies side-show. :) > > 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. Which system library headers require others? If you mean system headers as-in kernel headers, that's not the same league as application headers, and that does not discount the problems with the approach. > 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. Right, and this header may be an exception to the general rule in our tree.... but if all it takes are some forward declarations, then.... well, it seems silly not to put them there and let the bugger compile. Anyway, where is this information going to be documented in OpenOCD? This policy adds an extra cost for new developers, with little gain. While decoupling should be maximized, we are not rebuilding the kernel here, and -- as an application -- we should err on the side of being developer-friendly (in this case). I still want to be highly decoupled, and Jim seems like the biggest remaining barrier to solving that problem. > > 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. I am convinced that cleaning-up Jim would warrant a major revision bump for that library. (I'd kill the MixedCaps crap, if I had the chance.). The new help/usage commands deserve to be integrated with it directly, but the code needs to be scrubbed top-to-bottom with eye-bleach first. > It'll take quite a while to sort it all out cleanly though... Man-months of labor, if we want to do it without breaking things badly. --Z _______________________________________________ Openocd-development mailing list [email protected] https://lists.berlios.de/mailman/listinfo/openocd-development
