Yet another gcc warning flag.
Created attachment 9802 [details] patch Attaching patch with ChangeLogs. All of the changes to the code are making explicit, already implicit conversions/casts from 64 bit types (such as double or long long) to 32 bit types. Some of these conversions are worrisome as data is potentially being lost.
I'd rather first see a patch that has all non-debatable changes -- not including the typecasts that might be truncating values.
Created attachment 9834 [details] patch of trivial changes This patch includes only the trivial changes needed to compile with -Wshorten-64-to-32. The changes include adding 'f's to the end of float literals and switching some cos()'s and ceil()'s to cosf()'s and ceilf()'s. I've also added the flag to the Xcode project for JaveScriptGlue as it needs no modifications to compile with it.
Comment on attachment 9834 [details] patch of trivial changes r=me
Created attachment 9837 [details] patch of remaining less trivial changes This third patch contains the remaining changes. NOTE: none of the changes in this patch change any behavior, they simply explicitly down cast 64 bit types to 32 bit types that would be implicitly down cast anyway. The question is if those implicit conversions were safe to begin with and if not how we should deal with them.
Comment on attachment 9837 [details] patch of remaining less trivial changes The ones in JSNodeList are OK, because that's just test code. So we can definitely land the JavaScriptCore part of this. The one in WebFrameBridge.m looks right, so we can do the cast there. The one in WebNodeHighlightView.m looks fine. I'm not sure about the rest.
Created attachment 9851 [details] combind patch This patch includes all the trivial changes and the changes approved by Darin. Because some of the changes in WebKit are still under debate, I cannot turn on the -Wshorten-64-to-32 warning for it. The warning is on for WebCore, JavaScriptCore, and JavaScriptGlue though.
Comment on attachment 9851 [details] combind patch r=me
Comment on attachment 9837 [details] patch of remaining less trivial changes Clearing the review flag on this for now. We should go over the other cases in WebKit and see what we can do for each one. Either there's some reason we can guarantee the value fits in 32 bits or we need some checks and error handling.
Darin, What do you suggest I do in the mean time. Should I land the combined patch as is and open a new bug for the remaining cases, or should I wait until the rest is worked out?
(In reply to comment #10) > What do you suggest I do in the mean time. Should I land the combined patch as > is and open a new bug for the remaining cases, or should I wait until the rest > is worked out? You should definitely land the combined patch, and then we should work on the remaining cases. I'm not sure we need a new bug report, though. I'd be OK with landing the combined patch, clearing the review flag, and leaving this one open.
Comment on attachment 9851 [details] combind patch Landed the combined patch in r15799. I am clearing the review flag so we can continue to use this bug report.
Created attachment 9882 [details] remaining cases patch I'm posting what a patch with the code necessary to get WebKit to compile with the flag, but it is not intended for real use, only to highlight the remaining cases.
Only WebKit remains with this issue: // FIXME: <rdar://problem/5070292> WebKit should build with -Wshorten-64-to-32
But: 1) Newer versions of gcc warn a lot more with this warning and we had to turn it off in some places because of that. and 2) Oliver Hunt found that the feature branch had a lot of float/double conversions that run afoul of this, so when we merge the feature branch we will "backslide" a little on this warning.
<rdar://problem/5070292>
On 32-bit today, there is only one issue left in WebHistory.mm. Not sure about 64-bit.