hi! ok, i'm late,. but better late than never...
Kai Backman wrote: > On 1/4/07, Hans-Joachim Lankenau <[EMAIL PROTECTED]> wrote: >> with cws pchfi04 it was planned to use non-dummy precompiled header in a >> number of modules that should benefit most. the precompiled_<module>.hxx >> were created with the simplistic approach to include all used header >> found in the module and strip those that hinder compilation. >> >> unfortunately, when taking a closer look, this isn't sufficient. > > First, before we go forward. I'm pretty sure pchfix04 still contains > issues and I agree that we need to look into them. That is the exact > reason why the feature is marked as EXPERIMENTAL in ./configure and > not enabled by default. > > However, finding these problems is significantly faster if it is > easy for everyone to -test- them, which means the CWS should be > integrated sooner with issues rather than later when it's totally > perfect. It is not turned on by default. You can build using both > ways. You RE guys hate gigantic CWS's, maybe we should save some fixes > for pchfix05 and later? as the windows nonpro build here uses pch by default to make sure to detect missing includes, it's at least mandatory not to break the build and have some somehow usable results. > > >> e.g. in sd/source/core/drawdoc.cxx you find these lines: >> >> ... >> #define ITEMID_SCRIPTSPACE EE_PARA_ASIANCJKSPACING >> #include <svx/scriptspaceitem.hxx> >> ... >> >> this include uses the define to pass the default parameter to >> >> SvxScriptSpaceItem( sal_Bool bOn = sal_False, >> const sal_uInt16 nId = ITEMID_SCRIPTSPACE ); > > In this particular case the need for ITEMID_SCRIPTSPACE can be > entirely avoided by removing the default values and instead passing in > the parameters explicitly. A quick look at the all the OOo code found > me 7 invocations of the SvxScriptSpaceItem constructor so it should be > pretty trivial. > > ... > > The bigger question is why are such constructs used in the first > place? Modifying a hxx file using a define is just plain bad > programming in most cases. In this case it's not even excused by a > desire to simulate templates. If a module wants to define a default > script space, why not just do: > > const sal_uInt16 kSD_DefaultScriptSpace = EE_PARA_ASIANCJKSPACING; > > ... > new SvxScriptSpaceItem(sal_False, kSD_DefaultScriptSpace); > > In sd this is even simpler as there is only a single use of the > constructor so we can just update that single invocation. agreed, it's a matter of our codebase. see #i73604# > > >> another point that worries me is the different list of includes >> depending on using pch or not. > > Yes! Headers that are order dependent are usually broken in more > than one ways anyway. Finding them by turning on pch would be a Good > Thing. though i'm not a c/c++ expert, i also agree on this one. > > >> at the moment, i'm not sure that this "all you can include" approach >> will come to an happy end. maybe cautiously selecting a set of includes >> for the precompile hxx would be more appropriate regarding the two >> issues i currently see... > > At this point we have 8645 #include statements in the the various > pch files. It took martink and me about a month to come up with this > list using our simple strategy. Who will do this cautious selection? > Would it perhaps be a better idea to include those fixes in pchfix05? due to the concept of using pch, these code issues are more likely to pop up with pch although they are not limited to pch and may happen in non pch build too. since there seems to be some agreement that these constructs are worth fixing, it raises the hope that it will be possible to go on with including a large set of files into pch without damaging the build by design. > > > On 1/5/07, Stephan Bergmann <[EMAIL PROTECTED]> wrote: >> > a very simple result is that developers using pch will break the build >> > of those not using pch without being able to notice if new includes are >> > required that already reside in the precompiled file. any c/c++ expert >> > may be able to construct more elaborate error cases caused by this... >> >> What is worse, there can be cases where both versions build fine but >> behave differently (e.g., h1.hxx declares f(Base*), h2.hxx declares >> f(Derived*), u.cxx calls f with a Derived* and without pch only sees >> h1.hxx but with pch sees both h1.hxx and h2.hxx). > > Ok. Setting aside the fact that this will happen without pch > (someone could still include h2.hxx), I don't know C++ well enough to > say if it's even legal C++ or just plain bad? > > Anyway, where is such a construct used in OOo and am I being totally > naive with my gut feeling to publicly mock the author? ;-) > > I mean, if you know you want to call f(Base*) why no say: > f(static_cast<Base*> derived) yet another example of questionable code... my current plan with pchgfix04 is to get it compile here on windows (wntmsci10 without warnings) and linux (unxlngi6.pro), hopefully almost reached now. then give it some automated testing to see how many obvious issues we hit. if this is not too bad (first tests don't indicate so) resync it to something current and get it integrated. of cause there will be more work to do before doing release builds with pch enabled ;) tschau... ause --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
