206321 2020-01-15 15:42:52 -0800 Pass JSToken by const reference 2020-01-16 10:15:13 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jbedard jbedard commit-queue ews-watchlist keith_miller mark.lam msaboff saam simon.fraser tzagallo webkit-bug-importer oldest_to_newest 1607206 0 jbedard 2020-01-15 15:42:52 -0800 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 1607208 1 387859 jbedard 2020-01-15 15:44:40 -0800 Created attachment 387859 Patch 1607250 2 387859 saam 2020-01-15 17:11:18 -0800 Comment on attachment 387859 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); 1607374 3 387859 saam 2020-01-16 00:20:03 -0800 Comment on attachment 387859 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 1607474 4 jbedard 2020-01-16 07:54:06 -0800 (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. 1607524 5 387859 commit-queue 2020-01-16 10:14:17 -0800 Comment on attachment 387859 Patch Clearing flags on attachment: 387859 Committed r254689: <https://trac.webkit.org/changeset/254689> 1607525 6 commit-queue 2020-01-16 10:14:19 -0800 All reviewed patches have been landed. Closing bug. 1607526 7 webkit-bug-importer 2020-01-16 10:15:13 -0800 <rdar://problem/58648149> 387859 2020-01-15 15:44:40 -0800 2020-01-16 10:14:17 -0800 Patch bug-206321-20200115154440.patch text/plain 3621 jbedard SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjU0NjUxKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBA CisyMDIwLTAxLTE1ICBKb25hdGhhbiBCZWRhcmQgIDxqYmVkYXJkQGFwcGxlLmNvbT4KKworICAg ICAgICBQYXNzIEpTVG9rZW4gYnkgY29uc3QgcmVmZXJlbmNlCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDYzMjEKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHBhcnNlci9QYXJzZXIuY3BwOgorICAgICAg ICAoSlNDOjpQYXJzZXI8TGV4ZXJUeXBlPjo6Y3JlYXRlQmluZGluZ1BhdHRlcm4pOiBQYXNzIEpT VG9rZW4gYnkgY29uc3QgcmVmZXJlbmNlLgorICAgICAgICAqIHBhcnNlci9QYXJzZXIuaDogRGl0 dG8uCisKIDIwMjAtMDEtMTUgIEFkcmlhbiBQZXJleiBkZSBDYXN0cm8gIDxhcGVyZXpAaWdhbGlh LmNvbT4KIAogICAgICAgICBPZmZsaW5lYXNtIHdhcm5pbmdzIHdpdGggbmV3ZXIgUnVieSB2ZXJz aW9ucwpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL3BhcnNlci9QYXJzZXIuY3BwCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJzZXIvUGFyc2VyLmNwcAkocmV2aXNp b24gMjU0NTc1KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL3BhcnNlci9QYXJzZXIuY3BwCSh3 b3JraW5nIGNvcHkpCkBAIC05NDAsNyArOTQwLDcgQEAgYm9vbCBQYXJzZXI8TGV4ZXJUeXBlPjo6 ZGVjbGFyZVJlc3RPck5vcgogfQogCiB0ZW1wbGF0ZSA8dHlwZW5hbWUgTGV4ZXJUeXBlPgotdGVt cGxhdGUgPGNsYXNzIFRyZWVCdWlsZGVyPiBUcmVlRGVzdHJ1Y3R1cmluZ1BhdHRlcm4gUGFyc2Vy PExleGVyVHlwZT46OmNyZWF0ZUJpbmRpbmdQYXR0ZXJuKFRyZWVCdWlsZGVyJiBjb250ZXh0LCBE ZXN0cnVjdHVyaW5nS2luZCBraW5kLCBFeHBvcnRUeXBlIGV4cG9ydFR5cGUsIGNvbnN0IElkZW50 aWZpZXImIG5hbWUsIEpTVG9rZW4gdG9rZW4sIEFzc2lnbm1lbnRDb250ZXh0IGJpbmRpbmdDb250 ZXh0LCBjb25zdCBJZGVudGlmaWVyKiogZHVwbGljYXRlSWRlbnRpZmllcikKK3RlbXBsYXRlIDxj bGFzcyBUcmVlQnVpbGRlcj4gVHJlZURlc3RydWN0dXJpbmdQYXR0ZXJuIFBhcnNlcjxMZXhlclR5 cGU+OjpjcmVhdGVCaW5kaW5nUGF0dGVybihUcmVlQnVpbGRlciYgY29udGV4dCwgRGVzdHJ1Y3R1 cmluZ0tpbmQga2luZCwgRXhwb3J0VHlwZSBleHBvcnRUeXBlLCBjb25zdCBJZGVudGlmaWVyJiBu YW1lLCBjb25zdCBKU1Rva2VuJiB0b2tlbiwgQXNzaWdubWVudENvbnRleHQgYmluZGluZ0NvbnRl eHQsIGNvbnN0IElkZW50aWZpZXIqKiBkdXBsaWNhdGVJZGVudGlmaWVyKQogewogICAgIEFTU0VS VCghbmFtZS5pc051bGwoKSk7CiAgICAgCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcGFy c2VyL1BhcnNlci5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJzZXIv UGFyc2VyLmgJKHJldmlzaW9uIDI1NDU3NSkKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9wYXJz ZXIvUGFyc2VyLmgJKHdvcmtpbmcgY29weSkKQEAgLTE2MjksNyArMTYyOSw3IEBAIHByaXZhdGU6 CiAgICAgdGVtcGxhdGUgPGNsYXNzIFRyZWVCdWlsZGVyPiBUcmVlRXhwcmVzc2lvbiBwYXJzZVZh cmlhYmxlRGVjbGFyYXRpb25MaXN0KFRyZWVCdWlsZGVyJiwgaW50JiBkZWNsYXJhdGlvbnMsIFRy ZWVEZXN0cnVjdHVyaW5nUGF0dGVybiYgbGFzdFBhdHRlcm4sIFRyZWVFeHByZXNzaW9uJiBsYXN0 SW5pdGlhbGl6ZXIsIEpTVGV4dFBvc2l0aW9uJiBpZGVudFN0YXJ0LCBKU1RleHRQb3NpdGlvbiYg aW5pdFN0YXJ0LCBKU1RleHRQb3NpdGlvbiYgaW5pdEVuZCwgVmFyRGVjbGFyYXRpb25MaXN0Q29u dGV4dCwgRGVjbGFyYXRpb25UeXBlLCBFeHBvcnRUeXBlLCBib29sJiBmb3JMb29wQ29uc3REb2Vz Tm90SGF2ZUluaXRpYWxpemVyKTsKICAgICB0ZW1wbGF0ZSA8Y2xhc3MgVHJlZUJ1aWxkZXI+IFRy ZWVTb3VyY2VFbGVtZW50cyBwYXJzZUFycm93RnVuY3Rpb25TaW5nbGVFeHByZXNzaW9uQm9keVNv dXJjZUVsZW1lbnRzKFRyZWVCdWlsZGVyJik7CiAgICAgdGVtcGxhdGUgPGNsYXNzIFRyZWVCdWls ZGVyPiBUcmVlRXhwcmVzc2lvbiBwYXJzZUFycm93RnVuY3Rpb25FeHByZXNzaW9uKFRyZWVCdWls ZGVyJiwgYm9vbCBpc0FzeW5jKTsKLSAgICB0ZW1wbGF0ZSA8Y2xhc3MgVHJlZUJ1aWxkZXI+IE5F VkVSX0lOTElORSBUcmVlRGVzdHJ1Y3R1cmluZ1BhdHRlcm4gY3JlYXRlQmluZGluZ1BhdHRlcm4o VHJlZUJ1aWxkZXImLCBEZXN0cnVjdHVyaW5nS2luZCwgRXhwb3J0VHlwZSwgY29uc3QgSWRlbnRp ZmllciYsIEpTVG9rZW4sIEFzc2lnbm1lbnRDb250ZXh0LCBjb25zdCBJZGVudGlmaWVyKiogZHVw bGljYXRlSWRlbnRpZmllcik7CisgICAgdGVtcGxhdGUgPGNsYXNzIFRyZWVCdWlsZGVyPiBORVZF Ul9JTkxJTkUgVHJlZURlc3RydWN0dXJpbmdQYXR0ZXJuIGNyZWF0ZUJpbmRpbmdQYXR0ZXJuKFRy ZWVCdWlsZGVyJiwgRGVzdHJ1Y3R1cmluZ0tpbmQsIEV4cG9ydFR5cGUsIGNvbnN0IElkZW50aWZp ZXImLCBjb25zdCBKU1Rva2VuJiwgQXNzaWdubWVudENvbnRleHQsIGNvbnN0IElkZW50aWZpZXIq KiBkdXBsaWNhdGVJZGVudGlmaWVyKTsKICAgICB0ZW1wbGF0ZSA8Y2xhc3MgVHJlZUJ1aWxkZXI+ IE5FVkVSX0lOTElORSBUcmVlRGVzdHJ1Y3R1cmluZ1BhdHRlcm4gY3JlYXRlQXNzaWdubWVudEVs ZW1lbnQoVHJlZUJ1aWxkZXImLCBUcmVlRXhwcmVzc2lvbiYsIGNvbnN0IEpTVGV4dFBvc2l0aW9u JiwgY29uc3QgSlNUZXh0UG9zaXRpb24mKTsKICAgICB0ZW1wbGF0ZSA8Y2xhc3MgVHJlZUJ1aWxk ZXI+IE5FVkVSX0lOTElORSBUcmVlRGVzdHJ1Y3R1cmluZ1BhdHRlcm4gcGFyc2VPYmplY3RSZXN0 QmluZGluZ09yQXNzaWdubWVudEVsZW1lbnQoVHJlZUJ1aWxkZXImIGNvbnRleHQsIERlc3RydWN0 dXJpbmdLaW5kLCBFeHBvcnRUeXBlLCBjb25zdCBJZGVudGlmaWVyKiogZHVwbGljYXRlSWRlbnRp ZmllciwgQXNzaWdubWVudENvbnRleHQgYmluZGluZ0NvbnRleHQpOwogICAgIHRlbXBsYXRlIDxj bGFzcyBUcmVlQnVpbGRlcj4gTkVWRVJfSU5MSU5FIFRyZWVEZXN0cnVjdHVyaW5nUGF0dGVybiBw YXJzZUJpbmRpbmdPckFzc2lnbm1lbnRFbGVtZW50KFRyZWVCdWlsZGVyJiBjb250ZXh0LCBEZXN0 cnVjdHVyaW5nS2luZCwgRXhwb3J0VHlwZSwgY29uc3QgSWRlbnRpZmllcioqIGR1cGxpY2F0ZUlk ZW50aWZpZXIsIGJvb2wqIGhhc0Rlc3RydWN0dXJpbmdQYXR0ZXJuLCBBc3NpZ25tZW50Q29udGV4 dCBiaW5kaW5nQ29udGV4dCwgaW50IGRlcHRoKTsK