Closed
Bug 869238
Opened 12 years ago
Closed 12 years ago
TestCasting.cpp -Wtype-limit warnings
Categories
(Core :: MFBT, defect)
Core
MFBT
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: Waldo, Assigned: Waldo)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
3.71 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #746136 -
Flags: review?(nfroyd)
Updated•12 years ago
|
![]() |
||
Updated•12 years ago
|
Attachment #746136 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 2•12 years ago
|
||
Target Milestone: --- → mozilla23
![]() |
||
Comment 3•12 years ago
|
||
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.
Assignee | ||
Comment 4•12 years ago
|
||
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.
Assignee | ||
Comment 5•12 years ago
|
||
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.
Comment 6•12 years ago
|
||
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.
Description
•