Bug 109792 - Vector<T, inlineCapacity>::data() with size 0, returns null when inlineCapacity=0 and !null otherwise
Summary: Vector<T, inlineCapacity>::data() with size 0, returns null when inlineCapaci...
Status: RESOLVED LATER
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-14 01:22 PST by Eric Seidel (no email)
Modified: 2022-08-06 14:22 PDT (History)
10 users (show)

See Also:


Attachments
Patch (5.19 KB, patch)
2013-02-15 21:47 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2013-02-14 01:22:18 PST
Vector<T, inlineCapacity>::data() with size 0, returns null when inlineCapacity=0 and !null otherwise

This is inconsistent, and causes confusion when converting Vector<UChar> to Strings.  See bug 109784
Comment 1 Eric Seidel (no email) 2013-02-14 01:30:29 PST
This should be relatively easy to solve, we just have to be more careful to set m_buffer to 0 in the inlineCapacity case when size == 0.
Comment 2 Eric Seidel (no email) 2013-02-14 01:31:55 PST
Of course, once we fix this, we may need to fix bug 109784 in a different way for the threaded parser, but that's easy to do. :)
Comment 3 Maciej Stachowiak 2013-02-14 01:46:42 PST
What do you think the correct behavior should be, always null or never null?
Comment 4 Benjamin Poulain 2013-02-14 01:49:28 PST
(In reply to comment #3)
> What do you think the correct behavior should be, always null or never null?

I think undefined behavior could be okay, but always null would be more useful in practice.
Comment 5 Maciej Stachowiak 2013-02-14 02:02:43 PST
Looking at bug 109784, it seems to me that the right behavior for creating a String from an empty Vector is to get an empty string, not a null string. I am not sure if that has implications for this Vector issue.

Originally the design intent of Vector was that you should not rely on data() when length() is 0.

Making data() be always null is possible, but taking a quick look at the code, it would add extra branches to a number of VectorBase methods for the non-inline case (or else a branch in data() itself).
Comment 6 Benjamin Poulain 2013-02-14 02:10:12 PST
(In reply to comment #5)
> Looking at bug 109784, it seems to me that the right behavior for creating a String from an empty Vector is to get an empty string, not a null string. I am not sure if that has implications for this Vector issue.

I made a comment on that bug regarding that.

> Originally the design intent of Vector was that you should not rely on data() when length() is 0.
> 
> Making data() be always null is possible, but taking a quick look at the code, it would add extra branches to a number of VectorBase methods for the non-inline case (or else a branch in data() itself).

Does making it null in debug sounds like an acceptable compromise to you?

I would prefer it if people could not rely on that behavior, like in the case of Vector->String.
Comment 7 Benjamin Poulain 2013-02-15 21:47:41 PST
Created attachment 188691 [details]
Patch
Comment 8 Benjamin Poulain 2013-02-15 21:49:04 PST
Quick stab at it, it probably does not compile.
I think we may be able to make the change while reducing the number of branches.
Comment 9 Ahmad Saleem 2022-08-06 06:50:35 PDT
I can still see references to this in the Webkit Github source e.g.:

https://github.com/WebKit/WebKit/blob/f407fbb287465a0ff68442eb7297862518e211cc/Source/WTF/wtf/Vector.h#L427

I think this patch need to rebased to land. Appreciate if someone is willing to work else I think we can mark this as "RESOLVED LATER". Thanks!
Comment 10 Ryosuke Niwa 2022-08-06 14:22:06 PDT
Later.