Bug 234855 – Add FixedVector::clear(), contains(), find(), and findMatching().

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

This is needed so that we can use FixedVector in DeferredWorkTimer (see https://bugs.webkit.org/show_bug.cgi?id=234628#c2).

[Mark Lam]

Created attachment 448318 [details]
proposed patch.

[Yusuke Suzuki]

Comment on attachment 448318 [details]
proposed patch.

r=me

[Geoffrey Garen]

Comment on attachment 448318 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=448318&action=review

> Source/WTF/wtf/FixedVector.h:186
> +size_t FixedVector<T>::findMatching(const MatchFunction& matches) const

FWIW, the standard library calls this operation "findIf", which I find slightly clearer. ("Matching" sort of implies that you are searching for 'matches' as an entry in the subject vector.)

[Mark Lam]

(In reply to Geoffrey Garen from comment #3)
> Comment on attachment 448318 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=448318&action=review
> 
> > Source/WTF/wtf/FixedVector.h:186
> > +size_t FixedVector<T>::findMatching(const MatchFunction& matches) const
> 
> FWIW, the standard library calls this operation "findIf", which I find
> slightly clearer. ("Matching" sort of implies that you are searching for
> 'matches' as an entry in the subject vector.)

The findMatching name is a pre-existing one from WTF::Vector, and FixedVector aims to match Vector's API.  It looks like it is used all over the place.  I'll leave it as is for now so that we don't pollute this patch.  We can do a global rename in a separate patch later.

[Mark Lam]

(In reply to Mark Lam from comment #4)
> The findMatching name is a pre-existing one from WTF::Vector, and
> FixedVector aims to match Vector's API.  It looks like it is used all over
> the place.  I'll leave it as is for now so that we don't pollute this patch.
> We can do a global rename in a separate patch later.

Will do this renaming in https://bugs.webkit.org/show_bug.cgi?id=234864.

[Radar WebKit Bug Importer]

<rdar://problem/87412124>

[Mark Lam]

Comment on attachment 448318 [details]
proposed patch.

Thanks for the review.  Landing now.

[EWS]

Committed r288458 (246344@main): <https://commits.webkit.org/246344@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 448318 [details].

[Darin Adler]

Comment on attachment 448318 [details]
proposed patch.

I think that adding contains, findMatching, and find, are small steps in the wrong direction.

All of these work well on a Span, no need to have them as member functions on Vector. We need to find a way to not replicate the huge number of Vector functions all on FixedVector, by changing them to non-member functions with Span arguments. Both Vector and FixedVector can be efficiently changed into a Span, which can be thought of as a sort of VectorView.

If we convert most const functions in Vector to work on Span, then we can use them on C arrays (possibly), std::array, any Span, Vector, FixedVector, or even other things we invent in the future.

I think that eventually we should be doing that with one function at a time, rather than replicating all the Vector functions one at a time.