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?
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.
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.
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?
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)
Kai
--
Kai Backman, Software Engineer, [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
