Hi, I'm trying to separate the Support, System, ADT and Config header files into the support module, per previous plans. However, APFloat.h is not self-contained: APFloat.h:105:43: error: llvm/Bitcode/SerializationFwd.h: No such file or directory APFloat.h:106:37: error: llvm/CodeGen/ValueTypes.h: No such file or directory As you can see, APFloat.h depends on things that are not in the ADT, Support, or System header files. This, in my mind, constitutes a design flaw. Can we move the functionality that depends on these header files elsewhere? Reid.
On Dec 8, 2007, at 1:13 AM, Reid Spencer wrote:> Hi, > > I'm trying to separate the Support, System, ADT and Config header > files > into the support module, per previous plans. However, APFloat.h is not > self-contained: > > APFloat.h:105:43: error: llvm/Bitcode/SerializationFwd.h: No such file > or directory > APFloat.h:106:37: error: llvm/CodeGen/ValueTypes.h: No such file or > directory > > As you can see, APFloat.h depends on things that are not in the ADT, > Support, or System header files. This, in my mind, constitutes a > design > flaw. Can we move the functionality that depends on these header files > elsewhere?The inclusion of SerializationFwd.h can be removed by forward declaring the following two classes in APFloat.h: Serializer; Deserializer; If this doesn't seem like a clean enough solution, there are other ways to potentially decouple APFloat more from the Serialization "library." The tradeoff is that it would require some code rewriting/ addition, and could potentially make deserialization of APFloats (as done in the new C frontend) slightly less efficient.
> The inclusion of SerializationFwd.h can be removed by forward > declaring the following two classes in APFloat.h: > > Serializer; > Deserializer; > > If this doesn't seem like a clean enough solution, there are other > ways to potentially decouple APFloat more from the Serialization > "library." The tradeoff is that it would require some code rewriting/ > addition, and could potentially make deserialization of APFloats (as > done in the new C frontend) slightly less efficient.fixed -Chris
Hi Ted, On Sat, 2007-12-08 at 06:59 -0800, Ted Kremenek wrote:> On Dec 8, 2007, at 1:13 AM, Reid Spencer wrote: > > > Hi, > > > > I'm trying to separate the Support, System, ADT and Config header > > files > > into the support module, per previous plans. However, APFloat.h is not > > self-contained: > > > > APFloat.h:105:43: error: llvm/Bitcode/SerializationFwd.h: No such file > > or directory > > APFloat.h:106:37: error: llvm/CodeGen/ValueTypes.h: No such file or > > directory > > > > As you can see, APFloat.h depends on things that are not in the ADT, > > Support, or System header files. This, in my mind, constitutes a > > design > > flaw. Can we move the functionality that depends on these header files > > elsewhere? > > The inclusion of SerializationFwd.h can be removed by forward > declaring the following two classes in APFloat.h: > > Serializer; > Deserializer;Okay, but I imagine that leaves it still being used in APFloat.cpp which is also trying to be segregated to the support module.> > If this doesn't seem like a clean enough solution, there are other > ways to potentially decouple APFloat more from the Serialization > "library." The tradeoff is that it would require some code rewriting/ > addition, and could potentially make deserialization of APFloats (as > done in the new C frontend) slightly less efficient.I doubt the light performance loss will make any significant difference. Further, putting all the serialization code in one translation unit may have some benefit for a) clarity (one place to go instead of all over the place) and b) might perform better due to cache effects because it isn't spread all over the executable. Could you take the necessary steps to decouple APFloat from the serialization stuff and any other serialized data structures that live in lib/Support, lib/ADT or lib/System? That would help me get the separation going. While you're doing that, I'll be working on the makefile cleanup. Hopefully, soonish, we can have llvm module actually use the support module. Thanks, Reid.