type correctness and compilation speed


Subject: type correctness and compilation speed
From: Mike Nordell (tamlin@algonet.se)
Date: Tue Jun 19 2001 - 23:32:00 CDT


Correctness:
Something has seriously degraded lately, and that is type correctness.

Please, be aware that once you perform an arithmetic operation on a datatype
(such as operator+) you are indeed often creating a new dataype, e.g. short
+ int != short, why you can't really compare the result to a short.

Comparing an unsigned int and an int is in fact like comparing a pear and a
banana. -Just don't do it.

The LaTex importer still have unimplemented "color" and "font-family"
handling. If these compiler warnings indeed are intended because of this,
OK, but I think we wither implement it or #if 0 it. It really sucks to get
compiler warnings for something that has been left unimplemented for quite a
while now.

The Applix importer has to be fixed, either by removing it or by fixing its
logic where a switch has but one code path, "default:"!

fl_BlockLayout.cpp (line 5709 - it should be a law against having this long
source files!) is initializing a float from a double. fabs() AFAIK has never
returned float (at least not in ANSI times).

pf_Fragments.cpp(215) has what MSVC correctly defines an "empty controlled
statement". Either remove that code or fix it.

pp_Property.cpp(480) forces a variable to bool. Just fix.

The MSword importer has unused local variables.

The member function GR_Win32Graphics::_remapGlyphs() has seriously fucked
up, ehh. somewhat wrong, data types. A character offset could never be
negative here, could it, and "iLength" should sure as ... be a size_t.

UT_StringPtrMap::_key: "not all control paths return a value". Bloody bad if
you ask me.

Compilation speed:
Please, keep track of what you include into header files. Currently AbiWord
is such an entagled mess of includes that I can't build it _below_ five
minutes, while a year ago (I don't care of the "progress", it's not a part
of this) I couldn't get _above_ 90-120 seconds!

To give an example: why the ... should the event manager have _anything_
to do with iconv?!
Well, xap_EncodingManager is coded in a way that it includes "iconv.h", and
thereby _forcing anyone_ including that header file to _including_ the iconv
include dir into its "-I" compiler flags.

If possible (and accepted by the lot) I'd _really_ like if we started to use
"../../foo/bar.h" since this hierarchy is more or less cast in stone anyway.
It's not like anyone ever even _could_ take a directory like
"abi/src/af/xap/unix" and move it to another place and expect the makefiles
to still function. (the current style is hurting compilation performance,
that's why I mention it)

Yes, I'm just complaining without fixing any of this. I didn't write this to
get points, I wrote it to open eyes. If that makes me look bad, so be it; so
long as it enhances the quality of AbiWord.

/Mike - a quick Purify report to follow...



This archive was generated by hypermail 2b25 : Tue Jun 19 2001 - 23:35:35 CDT