Bug 196430 – Remove more i386 specific configurations

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

Remove more i386 specific configurations

[Sam Weinig]

Created attachment 366368 [details]
Patch

[Alexey Proskuryakov]

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?

[Sam Weinig]

(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?

[Alexey Proskuryakov]

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.

[Sam Weinig]

(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.

[Sam Weinig]

Created attachment 366371 [details]
Patch

[Alexey Proskuryakov]

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.

[WebKit Commit Bot]

Comment on attachment 366371 [details]
Patch

Clearing flags on attachment: 366371

Committed r243683: <https://trac.webkit.org/changeset/243683>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/49462324>