156012 2016-03-30 00:44:05 -0700 Make the $vm debugging tools available to builtins as @$vm. 2016-03-30 13:47:35 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Local Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 156026 1 mark.lam mark.lam benjamin commit-queue fpizlo ggaren joepeck keith_miller msaboff saam ysuzuki oldest_to_newest 1179003 0 mark.lam 2016-03-30 00:44:05 -0700 ... because we also need some debugging tools for builtin development. The $vm object will be made available to builtins as @dbg, which gives us, amongst many goodies, @dbg.print() (which prints the toString() values of its args) and @dbg.printValue() (which dataLogs its arg as a JSValue). @dbg will only be available if we run with JSC_enableDollarVM=true. Note: it would have been nice to use @$vm (instead of @dbg) but our CommonIdentifiers infrastructure does not current support $ in the name of identifiers (and it's not worth the hassle to add it). 1179067 1 ggaren 2016-03-30 09:07:52 -0700 Let's not use two names for the same object. 1179080 2 mark.lam 2016-03-30 09:55:39 -0700 (In reply to comment #1) > Let's not use two names for the same object. My options for keeping the name the same (where same means builtins would use the same name prepended by @) are: 1. Change the JSC CommonIdentifier infrastructure to be able to handle the $ character in the identifiers. 2. Rename $vm to something that CommonIdentifier can handle, e.g. _dbg. With that, the builtins can use @_dbg. The reason for choosing the $vm name in the first place was so that it has a low likelihood of collision with any names that a JS library may use. I also wanted a short name so that it's not too verbose to type. Given these needs, I think _dbg will fit the bill. A quick search of the web also shows no current popular use of _dbg in JS code. I'll go with option 2. Before proceeding with this patch, I'll make a separate patch to some renaming first: 1. $vm ==> _dbg 2. JSDollarVM ==> JSUnderscoreDbg. 3. JSDollarVMPrototype ==> JSUnderscoreDbgPrototype. 1179097 3 mark.lam 2016-03-30 10:38:09 -0700 (In reply to comment #2) > My options for keeping the name the same (where same means builtins would > use the same name prepended by @) are: > > 1. Change the JSC CommonIdentifier infrastructure to be able to handle the $ > character in the identifiers. > > 2. Rename $vm to something that CommonIdentifier can handle, e.g. _dbg. > With that, the builtins can use @_dbg. Correction: I have option 3: 3. Special case $vm in BuiltinNames.h. Option 3 is the simplest and least invasive to do. Moving forward with it. 1179108 4 275203 mark.lam 2016-03-30 11:01:11 -0700 Created attachment 275203 proposed patch. 1179119 5 275203 saam 2016-03-30 11:28:11 -0700 Comment on attachment 275203 proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=275203&action=review > Source/JavaScriptCore/builtins/BuiltinNames.h:72 > + const JSC::Identifier& dollarVMPublicName() const { return m_dollarVMName; } I don't think the public name is needed. It's also possible to use "dollarVM" as the identifier and not end up with all this duplicate code outside the macro. 1179172 6 mark.lam 2016-03-30 13:44:46 -0700 (In reply to comment #5) > Comment on attachment 275203 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=275203&action=review > > > Source/JavaScriptCore/builtins/BuiltinNames.h:72 > > + const JSC::Identifier& dollarVMPublicName() const { return m_dollarVMName; } > > I don't think the public name is needed. The lexer lowers @$vm to $vm and maps it the public name to the private name using the public to private map. The parser then checks for the private name in the private to public map. Hence, we do need both public and private names. > It's also possible to use "dollarVM" as the identifier and not end up with > all this duplicate code outside the macro. I think @$vm is a better choice because it's good to keep the way we reference the object the same for consistency. I'll go ahead and just land this patch. 1179173 7 275203 commit-queue 2016-03-30 13:47:31 -0700 Comment on attachment 275203 proposed patch. Clearing flags on attachment: 275203 Committed r198855: <http://trac.webkit.org/changeset/198855> 1179174 8 commit-queue 2016-03-30 13:47:35 -0700 All reviewed patches have been landed. Closing bug. 275203 2016-03-30 11:01:11 -0700 2016-03-30 13:47:31 -0700 proposed patch. bug-156012.patch text/plain 5926 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTk4ODQzKQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI5IEBA CisyMDE2LTAzLTMwICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBN YWtlIHRoZSAkdm0gZGVidWdnaW5nIHRvb2xzIGF2YWlsYWJsZSB0byBidWlsdGlucyBhcyBAJHZt LgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTU2MDEy CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2UgYWxz byBuZWVkIHNvbWUgZGVidWdnaW5nIHRvb2xzIGZvciBidWlsdGluIGRldmVsb3BtZW50LiAgVGhl ICR2bSBvYmplY3Qgd2lsbAorICAgICAgICBiZSBtYWRlIGF2YWlsYWJsZSB0byBidWlsdGlucyBh cyBAJHZtLCB3aGljaCBnaXZlcyB1cywgYW1vbmdzdCBtYW55IGdvb2RpZXMsCisgICAgICAgIEAk dm0ucHJpbnQoKSAod2hpY2ggcHJpbnRzIHRoZSB0b1N0cmluZygpIHZhbHVlcyBvZiBpdHMgYXJn cykgYW5kCisgICAgICAgIEAkdm0ucHJpbnRWYWx1ZSgpICh3aGljaCBkYXRhTG9ncyBpdHMgYXJn IGFzIGEgSlNWYWx1ZSkuICBAJHZtIHdpbGwgb25seSBiZQorICAgICAgICBhdmFpbGFibGUgaWYg d2UgcnVuIHdpdGggSlNDX3VzZURvbGxhclZNPXRydWUuCisKKyAgICAgICAgQWxzbyBjaGFuZ2Vk IEAkdm0ucHJpbnQoKSB0byBub3QgYXV0b21hdGljYWxseSBpbnNlcnQgYSBzcGFjZSBiZXR3ZWVu IHRoZQorICAgICAgICBwcmludGluZyBvZiBlYWNoIG9mIGl0cyBhcmdzLiAgVGhpcyBtYWtlcyBp dCBjbGVhcmVyIGFzIHRvIHdoYXQgd2lsbCBiZSBwcmludGVkCisgICAgICAgIGkuZS4gaXQgd2ls bCBvbmx5IHByaW50IHdoYXQgaXMgcGFzc2VkIHRvIGl0LgorCisgICAgICAgICogYnVpbHRpbnMv QnVpbHRpbk5hbWVzLmg6CisgICAgICAgIChKU0M6OkJ1aWx0aW5OYW1lczo6QnVpbHRpbk5hbWVz KToKKyAgICAgICAgKEpTQzo6QnVpbHRpbk5hbWVzOjpkb2xsYXJWTVB1YmxpY05hbWUpOgorICAg ICAgICAoSlNDOjpCdWlsdGluTmFtZXM6OmRvbGxhclZNUHJpdmF0ZU5hbWUpOgorICAgICAgICAq IHJ1bnRpbWUvSlNHbG9iYWxPYmplY3QuY3BwOgorICAgICAgICAoSlNDOjpKU0dsb2JhbE9iamVj dDo6aW5pdCk6CisgICAgICAgICogdG9vbHMvSlNEb2xsYXJWTVByb3RvdHlwZS5jcHA6CisgICAg ICAgIChKU0M6OmZ1bmN0aW9uUHJpbnQpOgorCiAyMDE2LTAzLTMwICBZdXN1a2UgU3V6dWtpICA8 dXRhdGFuZS50ZWFAZ21haWwuY29tPgogCiAgICAgICAgIFtKU0NdIEltcGxlbWVudCBTdHJpbmcu cHJvdG90eXBlLnJlcGVhdCBpbiBidWlsdGlucyBKUwpJbmRleDogU291cmNlL0phdmFTY3JpcHRD b3JlL2J1aWx0aW5zL0J1aWx0aW5OYW1lcy5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2Ny aXB0Q29yZS9idWlsdGlucy9CdWlsdGluTmFtZXMuaAkocmV2aXNpb24gMTk4ODQxKQorKysgU291 cmNlL0phdmFTY3JpcHRDb3JlL2J1aWx0aW5zL0J1aWx0aW5OYW1lcy5oCSh3b3JraW5nIGNvcHkp CkBAIC0zOSwxNiArMzksMjMgQEAgY2xhc3MgQnVpbHRpbk5hbWVzIHsKICAgICBXVEZfTUFLRV9O T05DT1BZQUJMRShCdWlsdGluTmFtZXMpOyBXVEZfTUFLRV9GQVNUX0FMTE9DQVRFRDsKICAgICAK IHB1YmxpYzoKKyAgICAvLyBXZSB0cmVhdCB0aGUgZG9sbGFyVk0gbmFtZSBhcyBhIHNwZWNpYWwg Y2FzZSBiZWxvdyBmb3IgJHZtIChiZWNhdXNlIENvbW1vbklkZW50aWZpZXJzIGRvZXMgbm90Cisg ICAgLy8geWV0IHN1cHBvcnQgdGhlICQgY2hhcmFjdGVyKS4KKwogICAgIEJ1aWx0aW5OYW1lcyhW TSogdm0sIENvbW1vbklkZW50aWZpZXJzKiBjb21tb25JZGVudGlmaWVycykKICAgICAgICAgOiBt X2VtcHR5SWRlbnRpZmllcihjb21tb25JZGVudGlmaWVycy0+ZW1wdHlJZGVudGlmaWVyKQogICAg ICAgICBKU0NfRk9SRUFDSF9CVUlMVElOX0ZVTkNUSU9OX05BTUUoSU5JVElBTElaRV9CVUlMVElO X05BTUVTKQogICAgICAgICBKU0NfQ09NTU9OX1BSSVZBVEVfSURFTlRJRklFUlNfRUFDSF9QUk9Q RVJUWV9OQU1FKElOSVRJQUxJWkVfQlVJTFRJTl9OQU1FUykKICAgICAgICAgSlNDX0NPTU1PTl9Q UklWQVRFX0lERU5USUZJRVJTX0VBQ0hfV0VMTF9LTk9XTl9TWU1CT0woSU5JVElBTElaRV9CVUlM VElOX1NZTUJPTFMpCisgICAgICAgICwgbV9kb2xsYXJWTU5hbWUoSWRlbnRpZmllcjo6ZnJvbVN0 cmluZyh2bSwgIiR2bSIpKQorICAgICAgICAsIG1fZG9sbGFyVk1Qcml2YXRlTmFtZShJZGVudGlm aWVyOjpmcm9tVWlkKFByaXZhdGVOYW1lKFByaXZhdGVOYW1lOjpEZXNjcmlwdGlvbiwgQVNDSUlM aXRlcmFsKCJQcml2YXRlU3ltYm9sLiR2bSIpKSkpCiAgICAgewogICAgICAgICBKU0NfRk9SRUFD SF9CVUlMVElOX0ZVTkNUSU9OX05BTUUoSU5JVElBTElaRV9QUklWQVRFX1RPX1BVQkxJQ19FTlRS WSkKICAgICAgICAgSlNDX0NPTU1PTl9QUklWQVRFX0lERU5USUZJRVJTX0VBQ0hfUFJPUEVSVFlf TkFNRShJTklUSUFMSVpFX1BSSVZBVEVfVE9fUFVCTElDX0VOVFJZKQogICAgICAgICBKU0NfRk9S RUFDSF9CVUlMVElOX0ZVTkNUSU9OX05BTUUoSU5JVElBTElaRV9QVUJMSUNfVE9fUFJJVkFURV9F TlRSWSkKICAgICAgICAgSlNDX0NPTU1PTl9QUklWQVRFX0lERU5USUZJRVJTX0VBQ0hfUFJPUEVS VFlfTkFNRShJTklUSUFMSVpFX1BVQkxJQ19UT19QUklWQVRFX0VOVFJZKQorICAgICAgICBtX3By aXZhdGVUb1B1YmxpY01hcC5hZGQobV9kb2xsYXJWTVByaXZhdGVOYW1lLmltcGwoKSwgJm1fZG9s bGFyVk1OYW1lKTsKKyAgICAgICAgbV9wdWJsaWNUb1ByaXZhdGVNYXAuYWRkKG1fZG9sbGFyVk1O YW1lLmltcGwoKSwgJm1fZG9sbGFyVk1Qcml2YXRlTmFtZSk7CiAgICAgfQogCiAgICAgYm9vbCBp c1ByaXZhdGVOYW1lKFN5bWJvbEltcGwmIHVpZCkgY29uc3Q7CkBAIC02MiwxMiArNjksMTYgQEAg cHVibGljOgogICAgIEpTQ19GT1JFQUNIX0JVSUxUSU5fRlVOQ1RJT05fTkFNRShERUNMQVJFX0JV SUxUSU5fSURFTlRJRklFUl9BQ0NFU1NPUikKICAgICBKU0NfQ09NTU9OX1BSSVZBVEVfSURFTlRJ RklFUlNfRUFDSF9QUk9QRVJUWV9OQU1FKERFQ0xBUkVfQlVJTFRJTl9JREVOVElGSUVSX0FDQ0VT U09SKQogICAgIEpTQ19DT01NT05fUFJJVkFURV9JREVOVElGSUVSU19FQUNIX1dFTExfS05PV05f U1lNQk9MKERFQ0xBUkVfQlVJTFRJTl9TWU1CT0xfQUNDRVNTT1IpCisgICAgY29uc3QgSlNDOjpJ ZGVudGlmaWVyJiBkb2xsYXJWTVB1YmxpY05hbWUoKSBjb25zdCB7IHJldHVybiBtX2RvbGxhclZN TmFtZTsgfQorICAgIGNvbnN0IEpTQzo6SWRlbnRpZmllciYgZG9sbGFyVk1Qcml2YXRlTmFtZSgp IGNvbnN0IHsgcmV0dXJuIG1fZG9sbGFyVk1Qcml2YXRlTmFtZTsgfQogCiBwcml2YXRlOgogICAg IElkZW50aWZpZXIgbV9lbXB0eUlkZW50aWZpZXI7CiAgICAgSlNDX0ZPUkVBQ0hfQlVJTFRJTl9G VU5DVElPTl9OQU1FKERFQ0xBUkVfQlVJTFRJTl9OQU1FUykKICAgICBKU0NfQ09NTU9OX1BSSVZB VEVfSURFTlRJRklFUlNfRUFDSF9QUk9QRVJUWV9OQU1FKERFQ0xBUkVfQlVJTFRJTl9OQU1FUykK ICAgICBKU0NfQ09NTU9OX1BSSVZBVEVfSURFTlRJRklFUlNfRUFDSF9XRUxMX0tOT1dOX1NZTUJP TChERUNMQVJFX0JVSUxUSU5fU1lNQk9MUykKKyAgICBjb25zdCBKU0M6OklkZW50aWZpZXIgbV9k b2xsYXJWTU5hbWU7CisgICAgY29uc3QgSlNDOjpJZGVudGlmaWVyIG1fZG9sbGFyVk1Qcml2YXRl TmFtZTsKICAgICB0eXBlZGVmIEhhc2hNYXA8UmVmUHRyPFVuaXF1ZWRTdHJpbmdJbXBsPiwgY29u c3QgSWRlbnRpZmllciosIElkZW50aWZpZXJSZXBIYXNoPiBCdWlsdGluTmFtZXNNYXA7CiAgICAg QnVpbHRpbk5hbWVzTWFwIG1fcHVibGljVG9Qcml2YXRlTWFwOwogICAgIEJ1aWx0aW5OYW1lc01h cCBtX3ByaXZhdGVUb1B1YmxpY01hcDsKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50 aW1lL0pTR2xvYmFsT2JqZWN0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENv cmUvcnVudGltZS9KU0dsb2JhbE9iamVjdC5jcHAJKHJldmlzaW9uIDE5ODg0MSkKKysrIFNvdXJj ZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTR2xvYmFsT2JqZWN0LmNwcAkod29ya2luZyBjb3B5 KQpAQCAtNjMwLDYgKzYzMCwxMiBAQCBwdXREaXJlY3RXaXRob3V0VHJhbnNpdGlvbih2bSwgdm0u cHJvcGVyCiAgICAgICAgIEpTRG9sbGFyVk1Qcm90b3R5cGUqIGRvbGxhclZNUHJvdG90eXBlID0g SlNEb2xsYXJWTVByb3RvdHlwZTo6Y3JlYXRlKHZtLCB0aGlzLCBKU0RvbGxhclZNUHJvdG90eXBl OjpjcmVhdGVTdHJ1Y3R1cmUodm0sIHRoaXMsIG1fb2JqZWN0UHJvdG90eXBlLmdldCgpKSk7CiAg ICAgICAgIG1fZG9sbGFyVk1TdHJ1Y3R1cmUuc2V0KHZtLCB0aGlzLCBKU0RvbGxhclZNOjpjcmVh dGVTdHJ1Y3R1cmUodm0sIHRoaXMsIGRvbGxhclZNUHJvdG90eXBlKSk7CiAgICAgICAgIEpTRG9s bGFyVk0qIGRvbGxhclZNID0gSlNEb2xsYXJWTTo6Y3JlYXRlKHZtLCBtX2RvbGxhclZNU3RydWN0 dXJlLmdldCgpKTsKKworICAgICAgICBHbG9iYWxQcm9wZXJ0eUluZm8gZXh0cmFTdGF0aWNHbG9i YWxzW10gPSB7CisgICAgICAgICAgICBHbG9iYWxQcm9wZXJ0eUluZm8odm0ucHJvcGVydHlOYW1l cy0+YnVpbHRpbk5hbWVzKCkuZG9sbGFyVk1Qcml2YXRlTmFtZSgpLCBkb2xsYXJWTSwgRG9udEVu dW0gfCBEb250RGVsZXRlIHwgUmVhZE9ubHkpLAorICAgICAgICB9OworICAgICAgICBhZGRTdGF0 aWNHbG9iYWxzKGV4dHJhU3RhdGljR2xvYmFscywgV1RGX0FSUkFZX0xFTkdUSChleHRyYVN0YXRp Y0dsb2JhbHMpKTsKKwogICAgICAgICBwdXREaXJlY3RXaXRob3V0VHJhbnNpdGlvbih2bSwgSWRl bnRpZmllcjo6ZnJvbVN0cmluZyhleGVjLCAiJHZtIiksIGRvbGxhclZNLCBEb250RW51bSk7CiAg ICAgfQogCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvdG9vbHMvSlNEb2xsYXJWTVByb3Rv dHlwZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL3Rvb2xzL0pTRG9s bGFyVk1Qcm90b3R5cGUuY3BwCShyZXZpc2lvbiAxOTg4NDEpCisrKyBTb3VyY2UvSmF2YVNjcmlw dENvcmUvdG9vbHMvSlNEb2xsYXJWTVByb3RvdHlwZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTMw MSw4ICszMDEsNiBAQCBzdGF0aWMgRW5jb2RlZEpTVmFsdWUgSlNDX0hPU1RfQ0FMTCBmdW5jCiBz dGF0aWMgRW5jb2RlZEpTVmFsdWUgSlNDX0hPU1RfQ0FMTCBmdW5jdGlvblByaW50KEV4ZWNTdGF0 ZSogZXhlYykKIHsKICAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgZXhlYy0+YXJndW1lbnRD b3VudCgpOyArK2kpIHsKLSAgICAgICAgaWYgKGkpCi0gICAgICAgICAgICBkYXRhTG9nKCIgIik7 CiAgICAgICAgIFN0cmluZyBhcmdTdHIgPSBleGVjLT51bmNoZWNrZWRBcmd1bWVudChpKS50b1N0 cmluZyhleGVjKS0+dmFsdWUoZXhlYyk7CiAgICAgICAgIGlmIChleGVjLT5oYWRFeGNlcHRpb24o KSkKICAgICAgICAgICAgIHJldHVybiBKU1ZhbHVlOjplbmNvZGUoanNVbmRlZmluZWQoKSk7Cg==