233335 2021-11-18 14:32:30 -0800 CellAttributes should be returned by value. 2021-11-18 16:23:20 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=233341 InRadar P2 Normal --- 232849 1 mark.lam mark.lam darin ews-watchlist keith_miller msaboff saam tzagallo webkit-bug-importer ysuzuki oldest_to_newest 1816633 0 mark.lam 2021-11-18 14:32:30 -0800 CellAttributes fits in 16 bits, and client code never modifies returned CellAttributes values. Hence, there is no reason to return them by reference. 1816635 1 webkit-bug-importer 2021-11-18 14:33:43 -0800 <rdar://problem/85568435> 1816638 2 444741 mark.lam 2021-11-18 14:40:24 -0800 Created attachment 444741 [fast-cq] proposed patch. 1816646 3 444741 ysuzuki 2021-11-18 14:52:31 -0800 Comment on attachment 444741 [fast-cq] proposed patch. r=me 1816651 4 444741 mark.lam 2021-11-18 15:07:25 -0800 Comment on attachment 444741 [fast-cq] proposed patch. Thanks for the review. 1816652 5 ews-feeder 2021-11-18 15:09:16 -0800 Committed r286033 (244421@main): <https://commits.webkit.org/244421@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 444741. 1816659 6 444741 darin 2021-11-18 15:38:50 -0800 Comment on attachment 444741 [fast-cq] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=444741&action=review > Source/JavaScriptCore/heap/BlockDirectory.h:77 > + const CellAttributes attributes() const { return m_attributes; } The const here in "const CellAttributes" doesn't provide much value, and should be removed. Same in all the other sites below. When returning a scalar, specifying const has almost no effect. We don’t label functions that return an int as returning a "const int". > Source/JavaScriptCore/heap/MarkedBlock.h:350 > + const CellAttributes attributes() const; Ditto. 1816676 7 mark.lam 2021-11-18 16:05:25 -0800 (In reply to Darin Adler from comment #6) > Comment on attachment 444741 [details] > [fast-cq] proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=444741&action=review > > > Source/JavaScriptCore/heap/BlockDirectory.h:77 > > + const CellAttributes attributes() const { return m_attributes; } > > The const here in "const CellAttributes" doesn't provide much value, and > should be removed. Same in all the other sites below. When returning a > scalar, specifying const has almost no effect. We don’t label functions that > return an int as returning a "const int". > > > Source/JavaScriptCore/heap/MarkedBlock.h:350 > > + const CellAttributes attributes() const; > > Ditto. I'll take care of this in an upcoming patch. 1816683 8 mark.lam 2021-11-18 16:23:07 -0800 (In reply to Mark Lam from comment #7) > (In reply to Darin Adler from comment #6) > > > Source/JavaScriptCore/heap/BlockDirectory.h:77 > > > + const CellAttributes attributes() const { return m_attributes; } > > > > The const here in "const CellAttributes" doesn't provide much value, and > > should be removed. Same in all the other sites below. When returning a > > scalar, specifying const has almost no effect. We don’t label functions that > > return an int as returning a "const int". > > I'll take care of this in an upcoming patch. Fixing this in https://bugs.webkit.org/show_bug.cgi?id=233341. 444741 2021-11-18 14:40:24 -0800 2021-11-18 15:09:17 -0800 [fast-cq] proposed patch. bug-233335.patch text/plain 6770 mark.lam SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjg2MDI5KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDI5IEBA CisyMDIxLTExLTE4ICBNYXJrIExhbSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBD ZWxsQXR0cmlidXRlcyBzaG91bGQgYmUgcmV0dXJuZWQgYnkgdmFsdWUuCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMzMzMzUKKyAgICAgICAgcmRhcjov Lzg1NTY4NDM1CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgQ2VsbEF0dHJpYnV0ZXMgZml0cyBpbiAxNiBiaXRzLCBhbmQgY2xpZW50IGNvZGUgbmV2ZXIg bW9kaWZpZXMgcmV0dXJuZWQgQ2VsbEF0dHJpYnV0ZXMKKyAgICAgICAgdmFsdWVzLiAgSGVuY2Us IHRoZXJlIGlzIG5vIHJlYXNvbiB0byByZXR1cm4gdGhlbSBieSByZWZlcmVuY2UuCisKKyAgICAg ICAgQWxzbyBmaXhlZCBhIGJpdC1yb3R0ZWQgY29tbWVudCBpbiBTdWJTcGFjZS5oLgorCisgICAg ICAgICogaGVhcC9CbG9ja0RpcmVjdG9yeS5oOgorICAgICAgICAoSlNDOjpCbG9ja0RpcmVjdG9y eTo6YXR0cmlidXRlcyBjb25zdCk6CisgICAgICAgICogaGVhcC9IZWFwQ2VsbFR5cGUuaDoKKyAg ICAgICAgKEpTQzo6SGVhcENlbGxUeXBlOjphdHRyaWJ1dGVzIGNvbnN0KToKKyAgICAgICAgKiBo ZWFwL01hcmtlZEJsb2NrLmg6CisgICAgICAgIChKU0M6Ok1hcmtlZEJsb2NrOjpIYW5kbGU6OmF0 dHJpYnV0ZXMgY29uc3QpOgorICAgICAgICAoSlNDOjpNYXJrZWRCbG9jazo6YXR0cmlidXRlcyBj b25zdCk6CisgICAgICAgICogaGVhcC9QcmVjaXNlQWxsb2NhdGlvbi5oOgorICAgICAgICAoSlND OjpQcmVjaXNlQWxsb2NhdGlvbjo6YXR0cmlidXRlcyBjb25zdCk6CisgICAgICAgICogaGVhcC9T dWJzcGFjZS5oOgorICAgICAgICAqIGhlYXAvU3Vic3BhY2VJbmxpbmVzLmg6CisgICAgICAgIChK U0M6OlN1YnNwYWNlOjphdHRyaWJ1dGVzIGNvbnN0KToKKwogMjAyMS0xMS0xOCAgWXVzdWtlIFN1 enVraSAgPHlzdXp1a2lAYXBwbGUuY29tPgogCiAgICAgICAgIFtKU0NdIEFkZCBicmFuY2hUZXN0 MTYgb3BlcmF0aW9uCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaGVhcC9CbG9ja0RpcmVj dG9yeS5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL0Jsb2NrRGly ZWN0b3J5LmgJKHJldmlzaW9uIDI4NjAyMikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFw L0Jsb2NrRGlyZWN0b3J5LmgJKHdvcmtpbmcgY29weSkKQEAgLTEsNSArMSw1IEBACiAvKgotICog Q29weXJpZ2h0IChDKSAyMDEyLTIwMTggQXBwbGUgSW5jLiBBbGwgcmlnaHRzIHJlc2VydmVkLgor ICogQ29weXJpZ2h0IChDKSAyMDEyLTIwMjEgQXBwbGUgSW5jLiBBbGwgcmlnaHRzIHJlc2VydmVk LgogICoKICAqIFJlZGlzdHJpYnV0aW9uIGFuZCB1c2UgaW4gc291cmNlIGFuZCBiaW5hcnkgZm9y bXMsIHdpdGggb3Igd2l0aG91dAogICogbW9kaWZpY2F0aW9uLCBhcmUgcGVybWl0dGVkIHByb3Zp ZGVkIHRoYXQgdGhlIGZvbGxvd2luZyBjb25kaXRpb25zCkBAIC03NCw3ICs3NCw3IEBAIHB1Ymxp YzoKICAgICB2b2lkIHNocmluaygpOwogICAgIHZvaWQgYXNzZXJ0Tm9VbnN3ZXB0KCk7CiAgICAg c2l6ZV90IGNlbGxTaXplKCkgY29uc3QgeyByZXR1cm4gbV9jZWxsU2l6ZTsgfQotICAgIGNvbnN0 IENlbGxBdHRyaWJ1dGVzJiBhdHRyaWJ1dGVzKCkgY29uc3QgeyByZXR1cm4gbV9hdHRyaWJ1dGVz OyB9CisgICAgY29uc3QgQ2VsbEF0dHJpYnV0ZXMgYXR0cmlidXRlcygpIGNvbnN0IHsgcmV0dXJu IG1fYXR0cmlidXRlczsgfQogICAgIGJvb2wgbmVlZHNEZXN0cnVjdGlvbigpIGNvbnN0IHsgcmV0 dXJuIG1fYXR0cmlidXRlcy5kZXN0cnVjdGlvbiA9PSBOZWVkc0Rlc3RydWN0aW9uOyB9CiAgICAg RGVzdHJ1Y3Rpb25Nb2RlIGRlc3RydWN0aW9uKCkgY29uc3QgeyByZXR1cm4gbV9hdHRyaWJ1dGVz LmRlc3RydWN0aW9uOyB9CiAgICAgSGVhcENlbGw6OktpbmQgY2VsbEtpbmQoKSBjb25zdCB7IHJl dHVybiBtX2F0dHJpYnV0ZXMuY2VsbEtpbmQ7IH0KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29y ZS9oZWFwL0hlYXBDZWxsVHlwZS5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29y ZS9oZWFwL0hlYXBDZWxsVHlwZS5oCShyZXZpc2lvbiAyODYwMjIpCisrKyBTb3VyY2UvSmF2YVNj cmlwdENvcmUvaGVhcC9IZWFwQ2VsbFR5cGUuaAkod29ya2luZyBjb3B5KQpAQCAtMSw1ICsxLDUg QEAKIC8qCi0gKiBDb3B5cmlnaHQgKEMpIDIwMTcgQXBwbGUgSW5jLiBBbGwgcmlnaHRzIHJlc2Vy dmVkLgorICogQ29weXJpZ2h0IChDKSAyMDE3LTIwMjEgQXBwbGUgSW5jLiBBbGwgcmlnaHRzIHJl c2VydmVkLgogICoKICAqIFJlZGlzdHJpYnV0aW9uIGFuZCB1c2UgaW4gc291cmNlIGFuZCBiaW5h cnkgZm9ybXMsIHdpdGggb3Igd2l0aG91dAogICogbW9kaWZpY2F0aW9uLCBhcmUgcGVybWl0dGVk IHByb3ZpZGVkIHRoYXQgdGhlIGZvbGxvd2luZyBjb25kaXRpb25zCkBAIC0zNiw3ICszNiw3IEBA IHB1YmxpYzoKICAgICBKU19FWFBPUlRfUFJJVkFURSBIZWFwQ2VsbFR5cGUoQ2VsbEF0dHJpYnV0 ZXMpOwogICAgIEpTX0VYUE9SVF9QUklWQVRFIHZpcnR1YWwgfkhlYXBDZWxsVHlwZSgpOwogCi0g ICAgY29uc3QgQ2VsbEF0dHJpYnV0ZXMmIGF0dHJpYnV0ZXMoKSBjb25zdCB7IHJldHVybiBtX2F0 dHJpYnV0ZXM7IH0KKyAgICBjb25zdCBDZWxsQXR0cmlidXRlcyBhdHRyaWJ1dGVzKCkgY29uc3Qg eyByZXR1cm4gbV9hdHRyaWJ1dGVzOyB9CiAKICAgICAvLyBUaGUgcHVycG9zZSBvZiBvdmVycmlk aW5nIHRoaXMgaXMgdG8gc3BlY2lhbGl6ZSB0aGUgc3dlZXAgZm9yIHlvdXIgZGVzdHJ1Y3RvcnMu IFRoaXMgd29uJ3QKICAgICAvLyBiZSBjYWxsZWQgZm9yIG5vLWRlc3RydWN0b3IgYmxvY2tzLiBU aGlzIG11c3QgY2FsbCBNYXJrZWRCbG9jazo6ZmluaXNoU3dlZXBLbm93aW5nU3Vic3BhY2UuCklu ZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENvcmUvaGVhcC9NYXJrZWRCbG9jay5oCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL01hcmtlZEJsb2NrLmgJKHJldmlzaW9uIDI4 NjAyMikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL01hcmtlZEJsb2NrLmgJKHdvcmtp bmcgY29weSkKQEAgLTE2MSw3ICsxNjEsNyBAQCBwdWJsaWM6CiAgICAgICAgIHNpemVfdCBjZWxs U2l6ZSgpOwogICAgICAgICBpbmxpbmUgdW5zaWduZWQgY2VsbHNQZXJCbG9jaygpOwogICAgICAg ICAKLSAgICAgICAgY29uc3QgQ2VsbEF0dHJpYnV0ZXMmIGF0dHJpYnV0ZXMoKSBjb25zdDsKKyAg ICAgICAgY29uc3QgQ2VsbEF0dHJpYnV0ZXMgYXR0cmlidXRlcygpIGNvbnN0OwogICAgICAgICBE ZXN0cnVjdGlvbk1vZGUgZGVzdHJ1Y3Rpb24oKSBjb25zdDsKICAgICAgICAgYm9vbCBuZWVkc0Rl c3RydWN0aW9uKCkgY29uc3Q7CiAgICAgICAgIEhlYXBDZWxsOjpLaW5kIGNlbGxLaW5kKCkgY29u c3Q7CkBAIC0zNDcsNyArMzQ3LDcgQEAgcHVibGljOgogICAgIHZvaWQgcmVzZXRBbGxvY2F0ZWQo KTsKICAgICAgICAgCiAgICAgc2l6ZV90IGNlbGxTaXplKCk7Ci0gICAgY29uc3QgQ2VsbEF0dHJp YnV0ZXMmIGF0dHJpYnV0ZXMoKSBjb25zdDsKKyAgICBjb25zdCBDZWxsQXR0cmlidXRlcyBhdHRy aWJ1dGVzKCkgY29uc3Q7CiAgICAgCiAgICAgYm9vbCBoYXNBbnlNYXJrZWQoKSBjb25zdDsKICAg ICB2b2lkIG5vdGVNYXJrZWQoKTsKQEAgLTUxMSwxMiArNTExLDEyIEBAIGlubGluZSBzaXplX3Qg TWFya2VkQmxvY2s6OmNlbGxTaXplKCkKICAgICByZXR1cm4gaGFuZGxlKCkuY2VsbFNpemUoKTsK IH0KIAotaW5saW5lIGNvbnN0IENlbGxBdHRyaWJ1dGVzJiBNYXJrZWRCbG9jazo6SGFuZGxlOjph dHRyaWJ1dGVzKCkgY29uc3QKK2lubGluZSBjb25zdCBDZWxsQXR0cmlidXRlcyBNYXJrZWRCbG9j azo6SGFuZGxlOjphdHRyaWJ1dGVzKCkgY29uc3QKIHsKICAgICByZXR1cm4gbV9hdHRyaWJ1dGVz OwogfQogCi1pbmxpbmUgY29uc3QgQ2VsbEF0dHJpYnV0ZXMmIE1hcmtlZEJsb2NrOjphdHRyaWJ1 dGVzKCkgY29uc3QKK2lubGluZSBjb25zdCBDZWxsQXR0cmlidXRlcyBNYXJrZWRCbG9jazo6YXR0 cmlidXRlcygpIGNvbnN0CiB7CiAgICAgcmV0dXJuIGhhbmRsZSgpLmF0dHJpYnV0ZXMoKTsKIH0K SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL1ByZWNpc2VBbGxvY2F0aW9uLmgKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvUHJlY2lzZUFsbG9jYXRpb24u aAkocmV2aXNpb24gMjg2MDIyKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvUHJlY2lz ZUFsbG9jYXRpb24uaAkod29ya2luZyBjb3B5KQpAQCAtMTIxLDcgKzEyMSw3IEBAIHB1YmxpYzoK ICAgICAgICAgcmV0dXJuIGFib3ZlTG93ZXJCb3VuZChyYXdQdHIpICYmIGJlbG93VXBwZXJCb3Vu ZChyYXdQdHIpOwogICAgIH0KICAgICAKLSAgICBjb25zdCBDZWxsQXR0cmlidXRlcyYgYXR0cmli dXRlcygpIGNvbnN0IHsgcmV0dXJuIG1fYXR0cmlidXRlczsgfQorICAgIGNvbnN0IENlbGxBdHRy aWJ1dGVzIGF0dHJpYnV0ZXMoKSBjb25zdCB7IHJldHVybiBtX2F0dHJpYnV0ZXM7IH0KICAgICAK ICAgICBEZXBlbmRlbmN5IGFib3V0VG9NYXJrKEhlYXBWZXJzaW9uKSB7IHJldHVybiBEZXBlbmRl bmN5KCk7IH0KICAgICAKSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL1N1YnNwYWNl LmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvU3Vic3BhY2UuaAko cmV2aXNpb24gMjg2MDIyKQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvU3Vic3BhY2Uu aAkod29ya2luZyBjb3B5KQpAQCAtMzksNyArMzksNyBAQCBjbGFzcyBIZWFwQ2VsbFR5cGU7CiAK IC8vIFRoZSBpZGVhIG9mIHN1YnNwYWNlcyBpcyB0aGF0IHlvdSBjYW4gcHJvdmlkZSBzb21lIGN1 c3RvbSBiZWhhdmlvciBmb3IgeW91ciBvYmplY3RzIGlmIHlvdQogLy8gYWxsb2NhdGUgdGhlbSBm cm9tIGEgY3VzdG9tIFN1YnNwYWNlIGluIHdoaWNoIHlvdSBvdmVycmlkZSBzb21lIG9mIHRoZSB2 aXJ0dWFsIG1ldGhvZHMuIFRoaXMKLS8vIGNsYXNzIGlzIHRoZSBiYXNlY2xhc3Mgb2YgU3Vic3Bh Y2VzLiBVc3VhbGx5IHlvdSB3aWxsIHVzZSBlaXRoZXIgU3Vic3BhY2Ugb3IgRml4ZWRTaXplU3Vi c3BhY2UuCisvLyBjbGFzcyBpcyB0aGUgYmFzZWNsYXNzIG9mIGFsbCBzdWJzcGFjZXMgZS5nLiBD b21wbGV0ZVN1YnNwYWNlLCBJc29TdWJzcGFjZS4KIGNsYXNzIFN1YnNwYWNlIHsKICAgICBXVEZf TUFLRV9OT05DT1BZQUJMRShTdWJzcGFjZSk7CiAgICAgV1RGX01BS0VfRkFTVF9BTExPQ0FURUQ7 CkBAIC01MCw3ICs1MCw3IEBAIHB1YmxpYzoKICAgICBjb25zdCBjaGFyKiBuYW1lKCkgY29uc3Qg eyByZXR1cm4gbV9uYW1lLmRhdGEoKTsgfQogICAgIE1hcmtlZFNwYWNlJiBzcGFjZSgpIGNvbnN0 IHsgcmV0dXJuIG1fc3BhY2U7IH0KICAgICAKLSAgICBjb25zdCBDZWxsQXR0cmlidXRlcyYgYXR0 cmlidXRlcygpIGNvbnN0OworICAgIGNvbnN0IENlbGxBdHRyaWJ1dGVzIGF0dHJpYnV0ZXMoKSBj b25zdDsKICAgICBIZWFwQ2VsbFR5cGUqIGhlYXBDZWxsVHlwZSgpIGNvbnN0IHsgcmV0dXJuIG1f aGVhcENlbGxUeXBlOyB9CiAgICAgQWxpZ25lZE1lbW9yeUFsbG9jYXRvciogYWxpZ25lZE1lbW9y eUFsbG9jYXRvcigpIGNvbnN0IHsgcmV0dXJuIG1fYWxpZ25lZE1lbW9yeUFsbG9jYXRvcjsgfQog ICAgIApJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvU3Vic3BhY2VJbmxpbmVzLmgK PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PQotLS0gU291cmNlL0phdmFTY3JpcHRDb3JlL2hlYXAvU3Vic3BhY2VJbmxpbmVz LmgJKHJldmlzaW9uIDI4NjAyMikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9oZWFwL1N1YnNw YWNlSW5saW5lcy5oCSh3b3JraW5nIGNvcHkpCkBAIC0xNTIsNyArMTUyLDcgQEAgdm9pZCBTdWJz cGFjZTo6Zm9yRWFjaExpdmVDZWxsKGNvbnN0IEZ1bgogICAgICAgICB9KTsKIH0KIAotaW5saW5l IGNvbnN0IENlbGxBdHRyaWJ1dGVzJiBTdWJzcGFjZTo6YXR0cmlidXRlcygpIGNvbnN0Citpbmxp bmUgY29uc3QgQ2VsbEF0dHJpYnV0ZXMgU3Vic3BhY2U6OmF0dHJpYnV0ZXMoKSBjb25zdAogewog ICAgIHJldHVybiBtX2hlYXBDZWxsVHlwZS0+YXR0cmlidXRlcygpOwogfQo=