Bug 12750 - Vector compare operator is broken
Summary: Vector compare operator is broken
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: All All
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2007-02-12 14:31 PST by Dex Deacon
Modified: 2007-02-13 10:43 PST (History)
1 user (show)

See Also:

patch for Vector comparison operators (2.87 KB, patch)
2007-02-12 18:12 PST, Dex Deacon
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dex Deacon 2007-02-12 14:31:54 PST
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.
Comment 1 Dex Deacon 2007-02-12 18:12:31 PST
Created attachment 13144 [details]
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.
Comment 2 Darin Adler 2007-02-12 18:15:06 PST
Comment on attachment 13144 [details]
patch for Vector comparison operators

Comment 3 Darin Adler 2007-02-12 18:15:33 PST
I also think we should remove the implicit cast.
Comment 4 Darin Adler 2007-02-13 10:43:21 PST
Committed revision 19606.