Make updates suggested by new version of Xcode
Created attachment 237744 [details] Patch
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Comment on attachment 237744 [details] Patch I'm not sure whether setting COMBINE_HIDPI_IMAGES = YES is compatible with our production builds. Can we leave that set to NO for projects that it wasn't already set in until we're able to determine for sure?
Created attachment 237746 [details] Patch
Committed r173364: <http://trac.webkit.org/changeset/173364>
EWS didn't approve of this patch, and it did break the build: /Volumes/Data/EWS/WebKit/Source/WebKit/mac/Misc/WebNSDataExtras.m:120:21: error: comparison of integers of different signs: 'typeof (length)' (aka 'unsigned long') and 'typeof (1024)' (aka 'int') [-Werror,-Wsign-compare] int remaining = MIN(length, WEB_GUESS_MIME_TYPE_PEEK_LENGTH) - (CHANNEL_TAG_LENGTH - 1); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Looks like a fix would not be mechanical.
CLANG_WARN_UNREACHABLE_CODE is causing issues in Release builds in JSC. /Users/Timothy/Work/Safari-TOT.git/OpenSource/Source/JavaScriptCore/bytecode/CodeBlock.cpp:282:14: error: code will never be executed [-Werror,-Wunreachable-code] /Users/Timothy/Work/Safari-TOT.git/OpenSource/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:126:24: error: code will never be executed [-Werror,-Wunreachable-code] /Users/Timothy/Work/Safari-TOT.git/OpenSource/Source/JavaScriptCore/dfg/DFGPlan.cpp:184:24: error: code will never be executed [-Werror,-Wunreachable-code] /Users/Timothy/Work/Safari-TOT.git/OpenSource/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:383:26: error: code will never be executed [-Werror,-Wunreachable-code] /Users/Timothy/Work/Safari-TOT.git/OpenSource/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:370:26: error: code will never be executed [-Werror,-Wunreachable-code] /Users/Timothy/Work/Safari-TOT.git/OpenSource/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:334:22: error: code will never be executed [-Werror,-Wunreachable-code] /Users/Timothy/Work/Safari-TOT.git/OpenSource/Source/JavaScriptCore/runtime/RegExp.cpp:276:19: error: code will never be executed [-Werror,-Wunreachable-code] /Users/Timothy/Work/Safari-TOT.git/OpenSource/Source/JavaScriptCore/runtime/RegExp.cpp:399:19: error: code will never be executed [-Werror,-Wunreachable-code] Seems to be mostly about RELEASE_ASSERT_NOT_REACHED().
(In reply to comment #6) > Looks like a fix would not be mechanical. We can just turn that warning off for that project.
(In reply to comment #7) > CLANG_WARN_UNREACHABLE_CODE is causing issues in Release builds in JSC. Should turn that warning off for that project.
(In reply to comment #6) > EWS didn't approve of this patch, and it did break the build: Maybe only on some versions of the compiler. Did not break the build on my system. Despite not seeing the problem locally, I am pretty sure that adding a U after 1024 would fix the bug without turning off any warnings: #define WEB_GUESS_MIME_TYPE_PEEK_LENGTH 1024U
(In reply to comment #9) > (In reply to comment #7) > > CLANG_WARN_UNREACHABLE_CODE is causing issues in Release builds in JSC. > > Should turn that warning off for that project. There’s also unreachable code in 32-bit builds of WTF and any other project that includes BitVector.h.
> adding a U after 1024 would fix the bug without turning off any warnings That should make MIN compile, but we'd still have an unchecked cast from the result of MIN to a signed int. While I'm not sure whether that would break the build anywhere, it's clearly wrong.
(In reply to comment #12) > > adding a U after 1024 would fix the bug without turning off any warnings > > That should make MIN compile, but we'd still have an unchecked cast from the result of MIN to a signed int. While I'm not sure whether that would break the build anywhere, it's clearly wrong. I don’t know what you mean when you say it’s wrong. It’s not wrong to cast something in the range 0-1024 to an int. I can’t think of any checking that would be needed. Maybe there’s better coding style, but there’s no correctness issue.
(In reply to comment #11) > There’s also unreachable code in 32-bit builds of WTF and any other project that includes BitVector.h. To me it seems fine to turn it off until we resolve those issues.
(In reply to comment #13) > (In reply to comment #12) > > > adding a U after 1024 would fix the bug without turning off any warnings > > > > That should make MIN compile, but we'd still have an unchecked cast from the result of MIN to a signed int. While I'm not sure whether that would break the build anywhere, it's clearly wrong. > > I don’t know what you mean when you say it’s wrong. It’s not wrong to cast something in the range 0-1024 to an int. I can’t think of any checking that would be needed. Maybe there’s better coding style, but there’s no correctness issue. Oh, I see, it’s not in the range 0-1024, but I still don’t see a correctness problem. Here’s the code: int remaining = MIN(length, WEB_GUESS_MIME_TYPE_PEEK_LENGTH) - (CHANNEL_TAG_LENGTH - 1); The MIN expression will be of type NSUInteger, CHANNEL_TAG_LENGTH is an int, as is 1, so we’re taking an NSUInteger in the range 0-1024, subtracting an int, and assigning to an int. I believe that is guaranteed to work. As I said, there may be a stylistic issue, but there is not a correctness issue.
I'm fixing the unreachable code cases on my (local, 64-bit) build. It might still be good to remove the warning in the interim. I'm introducing COMPILER_QUIRK(CONSIDERS_UNREACHABLE_CODE) since it's non-clang that is arguably wrong (by warning about problems that can happen on unreachable code paths). I will file a separate bug with that change.
See https://bugs.webkit.org/show_bug.cgi?id=136616
(In reply to comment #16) > I'm fixing the unreachable code cases on my (local, 64-bit) build. It might still be good to remove the warning in the interim. Great! > I'm introducing COMPILER_QUIRK(CONSIDERS_UNREACHABLE_CODE) since it's non-clang that is arguably wrong (by warning about problems that can happen on unreachable code paths). I will file a separate bug with that change. I like that. I should not have used !COMPILER(CLANG).