Bug 164844 - Web Inspector: Generator functions should have a displayable name when shown in stack traces
Summary: Web Inspector: Generator functions should have a displayable name when shown ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-11-16 17:13 PST by Joseph Pecoraro
Modified: 2016-11-18 10:57 PST (History)
13 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (20.45 KB, patch)
2016-11-16 17:29 PST, Joseph Pecoraro
ysuzuki: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-yosemite (1.96 MB, application/zip)
2016-11-16 18:50 PST, Build Bot
no flags Details
[PATCH] For Landing (21.56 KB, patch)
2016-11-17 15:13 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2016-11-16 17:13:24 PST
Summary:
Generator functions should have a displayable name when shown in stack traces.

Test:
<script>
function* anotherGenerator(i) {
    console.trace();
}

function* generator(i){
    yield* anotherGenerator(i);
}

generator(10).next();
</script>

Steps to Reproduce:
1. Open test case
2. View Console for trace() output
  => Unexpected "anonymous function"s

Safari:

    [Log] Trace
    	(anonymous function) (generator.html:6)
    	generatorResume
    	(anonymous function) (generator.html:12)
    	generatorResume
    	Global Code (generator.html:23)

Chrome:

    console.trace
        anotherGenerator @ generator.html:6

Firefox:

    console.trace()
        anotherGenerator() generator.html:6
        generator() generator.html:12
        <anonymous> generator.html:23

At the very least we should name the anonymous functions to match the generators that generated them, while being careful not to put an identifier name in the lexical scope. So I would expect something like this (and perhaps in the future eliminating the built-in functions):

    [Log] Trace
	anotherGenerator (generator.html:6)
	generatorResume
	generator (generator.html:12)
	generatorResume
	Global Code (generator.html:23)

Note:
• The same is true of async functions, but they also have other issues right now (they show up twice in the stack trace, so we should clean that up differently)
Comment 1 Radar WebKit Bug Importer 2016-11-16 17:24:58 PST
<rdar://problem/29300697>
Comment 2 Joseph Pecoraro 2016-11-16 17:29:24 PST
Created attachment 295002 [details]
[PATCH] Proposed Fix

There may be a better way of doing this. And eventually we'll probably want to do the same for Async functions. Lets see what the JSC folks think.
Comment 3 Joseph Pecoraro 2016-11-16 17:33:12 PST
Comment on attachment 295002 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=295002&action=review

> LayoutTests/inspector/debugger/js-stacktrace-expected.txt:148
> +        "functionName": "generator2",

Without this change all of these "generator#" strings would be the empty string, and show as (anonymous function) in Web Inspector.
Comment 4 Yusuke Suzuki 2016-11-16 17:40:05 PST
Comment on attachment 295002 [details]
[PATCH] Proposed Fix

Can you add a test to ensure the following case?

function* generator(factory)
{
    console.log(generator === factory); // should be true.
}
generator(generator).next();
Comment 5 Build Bot 2016-11-16 18:50:44 PST
Comment on attachment 295002 [details]
[PATCH] Proposed Fix

Attachment 295002 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2528657

New failing tests:
js/methods-names-should-not-be-in-lexical-scope.html
imported/w3c/web-platform-tests/custom-elements/attribute-changed-callback.html
fast/dom/MutationObserver/observe-exceptions.html
http/tests/websocket/tests/hybi/websocket-constructor-protocols.html
imported/w3c/web-platform-tests/custom-elements/CustomElementRegistry.html
js/class-syntax-method-names.html
fast/text/font-face-set-javascript.html
Comment 6 Build Bot 2016-11-16 18:50:47 PST
Created attachment 295014 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 7 Joseph Pecoraro 2016-11-17 15:07:06 PST
(In reply to comment #5)
> Comment on attachment 295002 [details]
> [PATCH] Proposed Fix
> 
> Attachment 295002 [details] did not pass mac-debug-ews (mac):
> Output: http://webkit-queues.webkit.org/results/2528657
> 
> New failing tests:
> js/methods-names-should-not-be-in-lexical-scope.html

These are real ASSERTs, I can't set a null string as the inferred name. I'll upload a new patch to let the bots re-run.
Comment 8 Joseph Pecoraro 2016-11-17 15:13:16 PST
Created attachment 295093 [details]
[PATCH] For Landing
Comment 9 Joseph Pecoraro 2016-11-18 10:57:49 PST
<https://trac.webkit.org/changeset/208885>