| Summary: | Remove more i386 specific configurations | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||||
| Component: | New Bugs | Assignee: | Sam Weinig <sam> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ap, commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Sam Weinig
2019-03-30 17:06:29 PDT
Created attachment 366368 [details]
Patch
Comment on attachment 366368 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366368&action=review > Source/JavaScriptCore/Configurations/Base.xcconfig:-84 > -GCC_WARN_64_TO_32_BIT_CONVERSION_i386 = YES; Don’t we still need this for sim? (In reply to Alexey Proskuryakov from comment #2) > Comment on attachment 366368 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=366368&action=review > > > Source/JavaScriptCore/Configurations/Base.xcconfig:-84 > > -GCC_WARN_64_TO_32_BIT_CONVERSION_i386 = YES; > > Don’t we still need this for sim? EWS seems to say no. Can it be trusted? You are just disabling a warning in this line, so what is your question exactly? If you can find watchOS Sim EWS bubble, it can be trusted... Wait, there is no watchOS Sim EWS! EWS has never been advertised as something that catches all possible mistakes for you. Everything else in your patch is Mac specific and no-op indeed, but not the removal of GCC_WARN_64_TO_32_BIT_CONVERSION_i386. (In reply to Alexey Proskuryakov from comment #4) > You are just disabling a warning in this line, so what is your question > exactly? Good point. In my mind, I made the assumption it would break the build. > > If you can find watchOS Sim EWS bubble, it can be trusted... Wait, there is > no watchOS Sim EWS! EWS has never been advertised as something that catches > all possible mistakes for you. > This seems a bit aggressive. > Everything else in your patch is Mac specific and no-op indeed, but not the > removal of GCC_WARN_64_TO_32_BIT_CONVERSION_i386. Fair enough, I will remove the non-mac specific changes. Created attachment 366371 [details]
Patch
Comment on attachment 366371 [details] Patch > This seems a bit aggressive. Correct. I think that it is well understood by experienced contributors that EWS is not supposed to catch every issue, so the comment about "trust" didn't sound like it was made in good faith. Comment on attachment 366371 [details] Patch Clearing flags on attachment: 366371 Committed r243683: <https://trac.webkit.org/changeset/243683> All reviewed patches have been landed. Closing bug. |