Closed Bug 869238 Opened 12 years ago Closed 12 years ago

TestCasting.cpp -Wtype-limit warnings

Categories

(Core :: MFBT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: Waldo, Assigned: Waldo)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

tbsaunde reported various -Wtype-limit compiler warnings in TestCasting.cpp when compiling with gcc 4.7. The warnings track back to various constructs in Casting.h that are sometimes vacuous, depending on how things are instantiated. Add more layers of template instantiation, or more-correct size checking, to fix this.
Attached patch PatchSplinter Review
Attachment #746136 - Flags: review?(nfroyd)
Blocks: buildwarning
Status: NEW → ASSIGNED
Version: unspecified → Trunk
Attachment #746136 - Flags: review?(nfroyd) → review+
Comment on attachment 746136 [details] [diff] [review] Patch Review of attachment 746136 [details] [diff] [review]: ----------------------------------------------------------------- ::: mfbt/Casting.h @@ +41,5 @@ > + > +template<typename From, typename To> > +struct UnsignedUnsignedCheck<From, To, FromIsBigger> > +{ > + public: Didn't catch this before, but I think we've said in the past that there's no need for |public:| in structs, as that's already the default. Fixing up the instances in this patch would be good; fixing the other instances in Casting.h can come along for the ride, if you like. Might merit a note in STYLE, too.
https://hg.mozilla.org/integration/mozilla-inbound/rev/883e35db4839 public-for-structs was, I think, for |struct S : T| where T would be public-inherited. Nobody's said anything yet for inside struct or class. :-) I know some people like EIBTI and so want public in struct, private in class, even when it's the default. We could make a rule, but it seems best to do that separately from this, given that we're not consistent either way.
Oh, the issue -- for future reference -- was that MSVC doesn't like sizeof(T) < sizeof(U) in template parameters -- it wants parentheses around it. No idea what the spec says, seems easy to add 'em, so I just did that.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: