12750 2007-02-12 14:31:54 -0800 Vector compare operator is broken 2007-02-13 10:43:21 -0800 1 1 1 Unclassified WebKit JavaScriptCore 420+ All All RESOLVED FIXED P2 Normal --- 1 occupant4 webkit-unassigned mjs oldest_to_newest 23712 0 occupant4 2007-02-12 14:31:54 -0800 Try running this code: Vector<char> a, b; a.fill('x', 10); b.fill('x', 10); ASSERT(a == b); The assertion will fail. There are two problems here: 1. The (templated) operator== for Vectors is defined as returning void, so it never actually gets instantiated! 2. Even if #1 is fixed, the operator== still isn't used by the compiler. Instead, the implicit cast to char* is being called (Vector<char>::operator char*()), and the data pointers are being compared. This affects a few other operator== functions that depend on Vector<T>::operator==, such as FormDataElement. The only reasonable solution I can think of is to remove the implicit cast, and just use the .data() accessor everywhere. Tested on Mac and Windows. 23666 1 13144 occupant4 2007-02-12 18:12:31 -0800 Created attachment 13144 patch for Vector comparison operators Actually, it turns out that after fixing #1, the implicit cast was being used only because the operator== was still broken in other ways. Specifically, it didn't accept const Vectors, so it wouldn't be used for all callers. After fixing Vector's operator==, I had to also add an operator!= for a class that previously wasn't being compared at all in RenderStyle.h. 23655 2 13144 darin 2007-02-12 18:15:06 -0800 Comment on attachment 13144 patch for Vector comparison operators r=me 23656 3 darin 2007-02-12 18:15:33 -0800 I also think we should remove the implicit cast. 23520 4 darin 2007-02-13 10:43:21 -0800 Committed revision 19606. 13144 2007-02-12 18:12:31 -0800 2007-02-12 18:15:06 -0800 patch for Vector comparison operators vector.patch text/plain 2935 occupant4 SW5kZXg6IEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBKYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgICAgKHJldmlzaW9uIDE5NTkzKQorKysgSmF2YVNjcmlwdENvcmUvQ2hh bmdlTG9nICAgICh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTUgQEAKKzIwMDctMDItMTIgIERl eCBEZWFjb24gIDxvY2N1cGFudDRAZ21haWwuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIC0gZml4IGZvciBodHRwOi8vYnVncy53ZWJraXQub3Jn L3Nob3dfYnVnLmNnaT9pZD0xMjc1MAorICAgICAgICBWZWN0b3Igb3BlcmF0b3I9PSB3YXMgbm90 IGRlZmluZWQgY29ycmVjdGx5LiAgSXQgcmV0dXJuZWQgdm9pZCwKKyAgICAgICAgZGlkIG5vdCBh Y2NlcHQgY29uc3QgVmVjdG9ycywgYW5kIHVzZWQgYW4gaW50IGluc3RlYWQgb2Ygc2l6ZV90LiAK KworICAgICAgICAqIHd0Zi9WZWN0b3IuaDogZml4ZWQgY29tcGFyaXNvbiBvcGVyYXRvcnMKKyAg ICAgICAgKFdURjo6b3BlcmF0b3I9PSk6CisgICAgICAgIChXVEY6Om9wZXJhdG9yIT0pOgorCiAy MDA3LTAyLTEwICBEYXZpZCBDYXJzb24gIDxkYWNhcnNvbkBnbWFpbC5jb20+CiAKICAgICAgICAg UmV2aWV3ZWQgYnkgTWFjaWVqLgpJbmRleDogSmF2YVNjcmlwdENvcmUvd3RmL1ZlY3Rvci5oCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIEphdmFTY3JpcHRDb3JlL3d0Zi9WZWN0b3IuaCAocmV2aXNpb24gMTk1OTMp CisrKyBKYXZhU2NyaXB0Q29yZS93dGYvVmVjdG9yLmggKHdvcmtpbmcgY29weSkKQEAgLTY2Mywx MiArNjYzLDEyIEBAIG5hbWVzcGFjZSBXVEYgewogICAgIH0KIAogICAgIHRlbXBsYXRlPHR5cGVu YW1lIFQsIHNpemVfdCBpbmxpbmVDYXBhY2l0eT4KLSAgICB2b2lkIG9wZXJhdG9yPT0oVmVjdG9y PFQsIGlubGluZUNhcGFjaXR5PiYgYSwgVmVjdG9yPFQsIGlubGluZUNhcGFjaXR5PiYgYikKKyAg ICBib29sIG9wZXJhdG9yPT0oY29uc3QgVmVjdG9yPFQsIGlubGluZUNhcGFjaXR5PiYgYSwgY29u c3QgVmVjdG9yPFQsIGlubGluZUNhcGFjaXR5PiYgYikKICAgICB7CiAgICAgICAgIGlmIChhLnNp emUoKSAhPSBiLnNpemUoKSkKICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKIAotICAgICAgICBm b3IgKGludCBpID0gMDsgaSA8IGEuc2l6ZSgpOyArK2kpCisgICAgICAgIGZvciAoc2l6ZV90IGkg PSAwOyBpIDwgYS5zaXplKCk7ICsraSkKICAgICAgICAgICAgIGlmIChhW2ldICE9IGJbaV0pCiAg ICAgICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogCkBAIC02NzYsNyArNjc2LDcgQEAgbmFtZXNw YWNlIFdURiB7CiAgICAgfQogCiAgICAgdGVtcGxhdGU8dHlwZW5hbWUgVCwgc2l6ZV90IGlubGlu ZUNhcGFjaXR5PgotICAgIGlubGluZSB2b2lkIG9wZXJhdG9yIT0oVmVjdG9yPFQsIGlubGluZUNh cGFjaXR5PiYgYSwgVmVjdG9yPFQsIGlubGluZUNhcGFjaXR5PiYgYikKKyAgICBpbmxpbmUgYm9v bCBvcGVyYXRvciE9KGNvbnN0IFZlY3RvcjxULCBpbmxpbmVDYXBhY2l0eT4mIGEsIGNvbnN0IFZl Y3RvcjxULCBpbmxpbmVDYXBhY2l0eT4mIGIpCiAgICAgewogICAgICAgICByZXR1cm4gIShhID09 IGIpOwogICAgIH0KSW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNv cmUvQ2hhbmdlTG9nICAgKHJldmlzaW9uIDE5NTkzKQorKysgV2ViQ29yZS9DaGFuZ2VMb2cgICAo d29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDA3LTAyLTEyICBEZXggRGVhY29uICA8 b2NjdXBhbnQ0QGdtYWlsLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICAtIGZpeCBmb3IgaHR0cDovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MTI3NTAKKyAgICAgICAgVmVjdG9yIG9wZXJhdG9yPT0gd2FzIG5vdCBkZWZpbmVkIGNv cnJlY3RseS4gIEl0IHJldHVybmVkIHZvaWQsCisgICAgICAgIGRpZCBub3QgYWNjZXB0IGNvbnN0 IFZlY3RvcnMsIGFuZCB1c2VkIGFuIGludCBpbnN0ZWFkIG9mIHNpemVfdC4gCisKKyAgICAgICAg KiByZW5kZXJpbmcvUmVuZGVyU3R5bGUuaDogYWRkZWQgYSAhPSBvcGVyYXRvciBTeWxlRGFzaGJv YXJkUmVnaW9uCisgICAgICAgIChXZWJDb3JlOjpTdHlsZURhc2hib2FyZFJlZ2lvbjo6b3BlcmF0 b3IhPSk6CisKIDIwMDctMDItMTIgIEtldmluIE1jQ3VsbG91Z2ggIDxrbWNjdWxsb3VnaEBhcHBs ZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgLgpJbmRleDogV2ViQ29yZS9yZW5kZXJpbmcv UmVuZGVyU3R5bGUuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJTdHls ZS5oIChyZXZpc2lvbiAxOTU5MykKKysrIFdlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclN0eWxlLmgg KHdvcmtpbmcgY29weSkKQEAgLTM4MSw2ICszODEsMTEgQEAgc3RydWN0IFN0eWxlRGFzaGJvYXJk UmVnaW9uIHsKICAgICB7CiAgICAgICAgIHJldHVybiB0eXBlID09IG8udHlwZSAmJiBvZmZzZXQg PT0gby5vZmZzZXQgJiYgbGFiZWwgPT0gby5sYWJlbDsKICAgICB9CisKKyAgIGJvb2wgb3BlcmF0 b3IhPShjb25zdCBTdHlsZURhc2hib2FyZFJlZ2lvbiYgbykgY29uc3QKKyAgIHsKKyAgICAgICBy ZXR1cm4gISgqdGhpcyA9PSBvKTsKKyAgIH0KIH07CiAKIC8vLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCg==