Bug 206321

Summary: Pass JSToken by const reference
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: JavaScriptCoreAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, simon.fraser, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Jonathan Bedard 2020-01-15 15:42:52 PST 
Simon shared shared a lgtm link, this was one of issues identified.

The JSToken object is larger than the word size, so we should really be passing it by const reference, not by value.

https://lgtm.com/projects/g/WebKit/webkit/?tag=&lang=cpp&mode=tree&ruleFocus=2163210742
Comment 1 Jonathan Bedard 2020-01-15 15:44:40 PST 
Created attachment 387859 [details]
Patch
Comment 2 Saam Barati 2020-01-15 17:11:18 PST 
Comment on attachment 387859 [details]
Patch

This call site concerns me since m_token is a field that's updated as we parse more things

    pattern = createBindingPattern(context, kind, exportType, *m_token.m_data.ident, m_token, bindingContext, duplicateIdentifier);
Comment 3 Saam Barati 2020-01-16 00:20:03 PST 
Comment on attachment 387859 [details]
Patch

Actually it looks like this function doesn’t lex, so we’re ok. Passing it by reference is ok since m_token won’t be updated

Can you verify also that it doesn’t end up lexing?

r=me
Comment 4 Jonathan Bedard 2020-01-16 07:54:06 PST 
(In reply to Saam Barati from comment #3)
> Comment on attachment 387859 [details]
> Patch
> 
> Actually it looks like this function doesn’t lex, so we’re ok. Passing it by
> reference is ok since m_token won’t be updated
> 
> Can you verify also that it doesn’t end up lexing?
> 
> r=me

It does not lex. And reading through what the function is doing, seems like it shouldn't be lexing in the future either, although this change would sort of force that on us.
Comment 5 WebKit Commit Bot 2020-01-16 10:14:17 PST 
Comment on attachment 387859 [details]
Patch

Clearing flags on attachment: 387859

Committed r254689: <https://trac.webkit.org/changeset/254689>
Comment 6 WebKit Commit Bot 2020-01-16 10:14:19 PST 
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2020-01-16 10:15:13 PST 
<rdar://problem/58648149>