clang fails to build khtml

[Michael Pyne]

On Wed, February 26, 2014 22:43:31 Milian Wolff wrote:
> But what about this:
> 
> In file included from src/ecma/kjs_traversal.cpp:21:
> src/kjs_traversal.lut.h:49:18: error: constant expression evaluates to
> 4294967295 which cannot be narrowed to type 'int' [-Wc++11-narrowing]
>    { "SHOW_ALL", DOM::NodeFilter::SHOW_ALL, KJS::DontDelete|KJS::ReadOnly,
> 0, 0 } ,
>                  ^~~~~~~~~~~~~~~~~~~~~~~~~
> src/kjs_traversal.lut.h:49:18: note: override this message by inserting an
> explicit cast
>    { "SHOW_ALL", DOM::NodeFilter::SHOW_ALL, KJS::DontDelete|KJS::ReadOnly,
> 0, 0 } ,
>                  ^~~~~~~~~~~~~~~~~~~~~~~~~
>                  static_cast<int>(        )
> 
> this actually sounds pretty dangerous to me. Anyone with knowledge around
> who knows what to do here?

I'm not sure of the type of DOM::NodeFilter::SHOW_ALL (Bernd or someone might 
know better) but it certainly seems like SHOW_ALL is supposed to be a constant 
equaling -1, which should be perfectly fine for an int.

What's probably happening is that the type is unsigned and SHOW_ALL is 
declared as the moral equivalent of (unsigned)-1 and clang rightly complains 
when trying to stuff that very large unsigned constant back into an int.

Makes me wonder if whatever this structure is can't simply use the right enum 
type here instead of int.

> There are also tons of warnings like this:
> 
> src/xml/dom_stringimpl.h:60:13: warning: cast from 'char *' to 'QChar *'
> increases required alignment from 1 to 2 [-Wcast-align]
>         s = (QChar*) new char[ sizeof(QChar)*( havestr ? len : 1 ) ];
>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> These can afaik also be "fixed" by using explicit static_casts. Is that OK?

Seems so. The standard I have says that static_cast<> is defined as long as 
the alignment is compatible, and it also says that new for arrays must return 
memory properly aligned for any object which could fit in the whole array, 
explicitly for this use case.

> What about this one:
> 
> src/frameworks/khtml/src/imload/decoders/jpegloader.cpp:430:29: warning:
> cast from 'uchar *' (aka 'unsigned char *') to 'QRgb *' (aka 'unsigned int
> *') increases required alignment from 1 to 4 [-Wcast-align]
>                 QRgb *out = (QRgb *)scanline;

Even assuming the alignment is not correct here (which is unlikely here given 
that the QRgb* seems to be going on multiples-of-4 here) then static_cast<> is 
at worst "unspecified", as opposed to "undefined behavior" where optimizers 
start adding buffer overflows. So it still seems like the better answer here 
unless we want to bring in some of those fancy image-swizzling graphics 
libraries.

Regards,
 - Michael Pyne