Open Bug 552793 Opened 15 years ago Updated 3 years ago

xpcom/tests/UTFStrings.h added 388 lines of build warnings "warning: overflow in implicit constant conversion"

Categories

(Core :: XPCOM, defect)

x86
Linux
defect

Tracking

()

People

(Reporter: dholbert, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [build_warning])

When building mozilla-central, I get 388 lines of build warnings in UTFStrings.h, all of which match one of the following: > xpcom/tests/UTFStrings.h:68: warning: overflow in implicit constant conversion > xpcom/tests/UTFStrings.h:90: warning: overflow in implicit constant conversion > xpcom/tests/UTFStrings.h:114: warning: overflow in implicit constant conversion (UTFStrings.h is included by both TestUTF.cpp and TestAtoms.cpp, and I get 194 lines of UTFStrings.h warnings for each of those .cpp files --> total of 388 warnings) Filing this bug on fixing this compiler-spam.
(er, sorry, meant to CC jonas, not to assign bug to him)
Assignee: jonas → nobody
I think we just need to change UTFStringsStringPair::m8 to be unsigned char.
That adds a ton of build errors, since everywhere we use *.m8 in TestUTF.cpp and TestAtoms.cpp, we expect a |const char*| -- not a |const unsigned char*|. In particular, the nsDependentCString constructor and do_GetAtom both expect a signed char pointer.
(In reply to comment #3) > That adds a ton of build errors, since everywhere we use *.m8 in TestUTF.cpp (by '*.m8' there I meant 'anything.m8' -- e.g. ValidStrings[i].m8)
I see three obvious simple solutions, each with downsides. (a) cast all of the out-of-bounds char values in UTFStrings.h to be unsigned (e.g. replace "0xDF" with "(unsigned char)0xDF"). - Downside: This makes UTFStrings.h much less readable. (b) Make UTFStringsStringPair::m8 unsigned, per comment 2, and cast it to be a char* everywhere that we use it. - Downside: this seems a bit hacky (c) Make do_GetAtom() and nsDependentCString() accept unsigned char arrays. - Downside: This would be a change to actual code, rather than just to tests. (but maybe it's a change we want?)
My vote goes towards (b) here. Could we use union tricks somehow? I forget if it's possible to do something like: struct UTFStringsStringPair { PRUnichar m16[16]; union { unsigned char m8tmp[16]; char m8[16]; }; };
(In reply to comment #6) > Could we use union tricks somehow? I don't think we can. I'm pretty sure that would prevent us from directly constructing UTFStringsStringPair structures inline, like we do in UTFStrings.h, e.g.: 70 static const UTFStringsStringPair Invalid16Strings[] = 71 { 72 { { 'a', 'b', 0xD800 }, 73 { 'a', 'b', 0xEF, 0xBF, 0xBD } }, In general, I don't think it's defined how to initialize a union from another value (or from an array, in this case).
Whiteboard: [build_warning]
Blocks: buildwarning
Component: String → XPCOM
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.