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)
Tracking
()
NEW
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.
Reporter | ||
Comment 1•15 years ago
|
||
(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.
Reporter | ||
Comment 3•15 years ago
|
||
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.
Reporter | ||
Comment 4•15 years ago
|
||
(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)
Reporter | ||
Comment 5•15 years ago
|
||
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];
};
};
Reporter | ||
Comment 7•15 years ago
|
||
(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).
Reporter | ||
Updated•15 years ago
|
Whiteboard: [build_warning]
Updated•14 years ago
|
Blocks: buildwarning
Assignee | ||
Updated•5 years ago
|
Component: String → XPCOM
Updated•3 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•