Patch coming.
Created attachment 295275 [details] proposed patch.
Comment on attachment 295275 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=295275&action=review > Source/JavaScriptCore/runtime/ProgramExecutable.cpp:82 > JSObject* exception = 0; nullptr > Source/JavaScriptCore/runtime/ScriptExecutable.cpp:314 > + ExecState* exec = scope->globalObject()->globalExec(); I would write: auto& state = *scope->globalObject()->globalExec(); > Source/JavaScriptCore/runtime/ScriptExecutable.cpp:318 > JSObject* exception = 0; nullptr
Comment on attachment 295275 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=295275&action=review >> Source/JavaScriptCore/runtime/ScriptExecutable.cpp:314 >> + ExecState* exec = scope->globalObject()->globalExec(); > > I would write: > > auto& state = *scope->globalObject()->globalExec(); I think making this change would go against the style of what we usually do inside JSC. I'm not against using this style throughout JSC, however, I think it's more important that we be consistent. I think at some point it's worth all of us having a more detailed discussion about coding style in JSC. I know that we mostly follow the normal WebKit style, however, I've noticed that we've differed from that style in some ways. Notably, I think JSC uses "auto" much less than the rest of WebKit does. In this specific line of code, I usually see it written as Mark has written it. We tend to call things of type "ExecState*" exec, and not declare it using auto.
Comment on attachment 295275 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=295275&action=review >>> Source/JavaScriptCore/runtime/ScriptExecutable.cpp:314 >>> + ExecState* exec = scope->globalObject()->globalExec(); >> >> I would write: >> >> auto& state = *scope->globalObject()->globalExec(); > > I think making this change would go against the style of what we usually do inside JSC. I'm not against using this style throughout JSC, however, I think it's more important that we be consistent. I think at some point it's worth all of us having a more detailed discussion about coding style in JSC. I know that we mostly follow the normal WebKit style, however, I've noticed that we've differed from that style in some ways. Notably, I think JSC uses "auto" much less than the rest of WebKit does. > > In this specific line of code, I usually see it written as Mark has written it. We tend to call things of type "ExecState*" exec, and not declare it using auto. I think I'll go ahead and cave, and switch to using the convention of using ExecState& state instead where possible since I'll be touching a lot of code in my exception checks series of patches. I don't see the WebKit project switching back to using "exec". So, might as well get started on the switch.
Thanks for the reviews. Landed in r208950: <http://trac.webkit.org/r208950>.