Bug 109792 – Vector<T, inlineCapacity>::data() with size 0, returns null when inlineCapacity=0 and !null otherwise

• Component: New Bugs
• Status: RESOLVED LATER

[Eric Seidel (no email)]

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

[Eric Seidel (no email)]

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.

[Eric Seidel (no email)]

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. :)

[Maciej Stachowiak]

What do you think the correct behavior should be, always null or never null?

[Benjamin Poulain]

(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.

[Maciej Stachowiak]

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).

[Benjamin Poulain]

(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.

[Benjamin Poulain]

Created attachment 188691 [details]
Patch

[Benjamin Poulain]

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.

[Ahmad Saleem]

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!

[Ryosuke Niwa]

Later.