- if not, how about a a one-off, push_back be added to WTF::Vector?Aleks
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAMdyzDvoGrOzXrbCPAmb4XXek6F8qLhmPsNRkuY%3DyT7%2BZACt%2Bw%40mail.gmail.com.
+1 to renaming WTF things to match STL ones, I find it confusing to have both.
On 20 October 2016 at 17:45, 'Aleks Totic' via platform-architecture-dev <platform-architecture-dev@chromium.org> wrote:
I've been working with WTF collections a lot recently.I often find myself imagining my code as STL, thenmapping the solution to WTF.WTF having different method names for same functionality causes me trouble.In particular, the way we append elements: WTF::Array::append vs std::array::push_back.- there is a mental cost of keeping track of two ways of doing it.- you can't use some of std::algorithm for common tasks.For example, back_inserter does not work, because it wants a collection that has push_back.Instead of idiomatic:std::copy (bar.begin(),bar.end(),back_inserter(foo));I have to write manual loops instead (except for Vectors, can use copyToVector).for (auto& n: bar)foo.push_back(n);My questions are:- is there any interest in alligning STL and WTF APIs?- if not, how about a a one-off, push_back be added to WTF::Vector?Aleks
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAMdyzDvoGrOzXrbCPAmb4XXek6F8qLhmPsNRkuY%3DyT7%2BZACt%2Bw%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAPG_qM6K6rRoYfhZQe%3D7rF%2BP%2BoAaHqbaGzsogsFr-FRmu3XSyg%40mail.gmail.com.
I think we should do this -- we should make WTF APIs more consistent with STL APIs.I don't think this is controversial, but the problem is how to find eng resources for the renaming work? :)
--Aleks
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAMdyzDv_ireSeU1LCB7i_7_WD2u5fr7E%2BZrM%3Dp3o-CG4YvL%3DEQ%40mail.gmail.com.
On Thu, Oct 20, 2016 at 11:18 PM 'Aleks Totic' via platform-architecture-dev <platform-architecture-dev@chromium.org> wrote:I think we should do this -- we should make WTF APIs more consistent with STL APIs.I don't think this is controversial, but the problem is how to find eng resources for the renaming work? :)That is really tempting, I love reducing the number of hops my brain has to make when writing code. I have 2 big things on my plate right now. Maybe after these are done. Any guess on how long might this take?I think we'd want to come up with a reasonable migration plan. Just adding push_back() is easy enough, but there are several thousand uses of append(U&&) that we should switch for consistency. We also have emplaceAppend(Args&&...), an overload of append(U*, size_t), etc, that we'd probably want to update as well (IMO, it'd be even more confusing if we ended up in a halfway state for an extended period of time).
Daniel
--Aleks
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAMdyzDv_ireSeU1LCB7i_7_WD2u5fr7E%2BZrM%3Dp3o-CG4YvL%3DEQ%40mail.gmail.com.
On Fri, Oct 21, 2016 at 7:50 AM, Daniel Cheng <dch...@chromium.org> wrote:On Thu, Oct 20, 2016 at 11:18 PM 'Aleks Totic' via platform-architecture-dev <platform-architecture-dev@chromium.org> wrote:I think we should do this -- we should make WTF APIs more consistent with STL APIs.I don't think this is controversial, but the problem is how to find eng resources for the renaming work? :)That is really tempting, I love reducing the number of hops my brain has to make when writing code. I have 2 big things on my plate right now. Maybe after these are done. Any guess on how long might this take?I think we'd want to come up with a reasonable migration plan. Just adding push_back() is easy enough, but there are several thousand uses of append(U&&) that we should switch for consistency. We also have emplaceAppend(Args&&...), an overload of append(U*, size_t), etc, that we'd probably want to update as well (IMO, it'd be even more confusing if we ended up in a halfway state for an extended period of time).+1 to do planning before starting random replacement, at least about Vector, HashTable and String.Is there any volunteer who is interested in this work?
WTF isn't used in content or chrome, what do you need to change there?
>
> More thoughts?
>
> -Mark
>
> --
> You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
> To post to this group, send email to platform-arc...@chromium.org.
> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/59335a56-39d3-47ec-a94d-6b175d6b1159%40chromium.org.
> To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
> To post to this group, send email to platform-architecture-dev@chromium.org.
> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/59335a56-39d3-47ec-a94d-6b175d6b1159%40chromium.org.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAO9Q3iKYbVh6QJd2z9_vB8W3UFvRGyTLGPKYUrqY070hkbtUnA%40mail.gmail.com.
Would it be possible to create a list of Vector's methods and a renaming plan for them? For example, isEmpty() => empty(), reserveCapacity() => reserve() etc.
I think we should leave isEmpty() as an alias of empty(). We use isEmpty all over and the isFoo() for boolean test methods is both common practice all over chromium and well understood. This is a place where the stl is weirdly named compared to most other languages and common practice.
> To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
> To post to this group, send email to platform-architecture-dev@chromium.org.
> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/59335a56-39d3-47ec-a94d-6b175d6b1159%40chromium.org.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAO9Q3iKYbVh6QJd2z9_vB8W3UFvRGyTLGPKYUrqY070hkbtUnA%40mail.gmail.com.
I'd love to see better Oilpan integration docs inside .md, lots of valuable info is only available in not-so-readable template files. See https://crbug.com/655787 for examples. I volunteer for this if you need help.Regarding Oilpan, we have a tutorial and a full documentation.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAD649j6tYc-3MzksJaTQjb9n2eTHHXg7VgG179xT2_6dXc-ybA%40mail.gmail.com.
I'm not opposed to renaming things in our WTF containers, but that would be tricky because we special-case a container containing some smart pointer like RefPtr<T>, std::unique_ptr<T> or Member<T>, which is basically treated as an array of raw pointers instead of the smart pointers. (This lets you look up a pointer in HashSet<std::unique_ptr>, which isn't easy with std containers.)I'm not sure whether renaming all the names around has justifiable merits (compared to the work amount), though. I consider WTF containers are different from std containers, and I think it's natural that we have different names for stuff with different functionalities. (And I feel uneasy when an non-std class provides only methods with a different naming convention; I need to switch my minds every time I deal with "Blink code but adopting std naming scheme", which costs me a significant cognitive burden. It's simple for me that all Blink things adopt the Blink style. I might be in the minority, though.)In this specific case, adding a std-compatible push_back() function would be sufficient to unblock the OP's problem, right?
Thanks,Yuta
On Tue, Oct 25, 2016 at 3:15 AM, Joshua Bell <jsb...@chromium.org> wrote:
On Sun, Oct 23, 2016 at 11:16 PM, 'Aleks Totic' via platform-architecture-dev <platform-architecture-dev@chromium.org> wrote:I'd love to see better Oilpan integration docs inside .md, lots of valuable info is only available in not-so-readable template files. See https://crbug.com/655787 for examples. I volunteer for this if you need help.Regarding Oilpan, we have a tutorial and a full documentation.Full documentation has an empty section for Heap Collections.I've been unable to find docs about:- What collections are available, and what do they do (or just a pointer to standards class they are modeled after).- Do they accept WeakMembers? What are the performance characteristics?- Other details of using Oilpan, mentioned in the bug.After working on Chrome for a while, I've gained a lot of this knowledge from verbal communication, and reading of header files and existing code. It was an inefficient process.Having ~complete docs for foundational classes would help all new Chrome developers. In a perfect world, every time I have to ask a question about foundational stuff, it should be answered by a link to an updated doc (or make it asker's job to update the docs).How about we (Mark?) create a spreadsheet, one sheet per class (?), list the current methods/overloads, note potential renames/etc for alignment with STL (soliciting comments), and have column(s) for documentation/examples and we crowd-source the docs?A willing volunteer would convert the results to Markdown when we're done.BTW, is this a comprehensive list of the WTF container types, or are there more lurking?BitVector Deque DoublyLinkedList HashCountedSet HashMap HashSet LinkedHashSet LinkedStack ListHashSet RefVector.h TerminatedArray(?) Vector
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAD649j6tYc-3MzksJaTQjb9n2eTHHXg7VgG179xT2_6dXc-ybA%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAFJcur9xpDWQrF5ZiG57s-Nb36gh4Fmi_RHbVHTH0FDOq-duKA%40mail.gmail.com.
On 25 October 2016 at 09:01, Yuta Kitamura <yu...@chromium.org> wrote:I'm not opposed to renaming things in our WTF containers, but that would be tricky because we special-case a container containing some smart pointer like RefPtr<T>, std::unique_ptr<T> or Member<T>, which is basically treated as an array of raw pointers instead of the smart pointers. (This lets you look up a pointer in HashSet<std::unique_ptr>, which isn't easy with std containers.)I'm not sure whether renaming all the names around has justifiable merits (compared to the work amount), though. I consider WTF containers are different from std containers, and I think it's natural that we have different names for stuff with different functionalities. (And I feel uneasy when an non-std class provides only methods with a different naming convention; I need to switch my minds every time I deal with "Blink code but adopting std naming scheme", which costs me a significant cognitive burden. It's simple for me that all Blink things adopt the Blink style. I might be in the minority, though.)In this specific case, adding a std-compatible push_back() function would be sufficient to unblock the OP's problem, right?I'd like to propose we initially add aliases for all WTF container methods that map 1:1 to stl functions e.g.WTF::Vector
- push_back = append
- erase = remove
- empty = isEmpty
- emplace_back = emplaceAppend
- pop_back = removeLast
- front = first
- back = last
WTF::HashMap
- insert = add
- erase = remove
- empty = isEmpty
- at = get
- (maybe) implement operator[]
WTF::HashSet
- insert = add
- erase = remove
- empty = isEmpty
There might be some I missed, but for me that would solve most of the problem. Of course we'd have two names for things. It would be possible to write a clang tool to rename everything, although that does seem to be quite a bit of work.On a side note I recently used WTF::HashMap with the key being a WTF::Vector<String> this was surprisingly difficult to write because it needed a custom hash traits class with lots of odd members. I note the STL also requires you to write a custom hash function and comparison for it's equivalent but that's very much easier to do. I don't have any specific proposal here, I appreciate the optimization that has gone into WTF::HashMap I just wish it was easier to write custom hash traits for it.
Thanks,Yuta
On Tue, Oct 25, 2016 at 3:15 AM, Joshua Bell <jsb...@chromium.org> wrote:
On Sun, Oct 23, 2016 at 11:16 PM, 'Aleks Totic' via platform-architecture-dev <platform-arc...@chromium.org> wrote:I'd love to see better Oilpan integration docs inside .md, lots of valuable info is only available in not-so-readable template files. See https://crbug.com/655787 for examples. I volunteer for this if you need help.Regarding Oilpan, we have a tutorial and a full documentation.Full documentation has an empty section for Heap Collections.I've been unable to find docs about:- What collections are available, and what do they do (or just a pointer to standards class they are modeled after).- Do they accept WeakMembers? What are the performance characteristics?- Other details of using Oilpan, mentioned in the bug.After working on Chrome for a while, I've gained a lot of this knowledge from verbal communication, and reading of header files and existing code. It was an inefficient process.Having ~complete docs for foundational classes would help all new Chrome developers. In a perfect world, every time I have to ask a question about foundational stuff, it should be answered by a link to an updated doc (or make it asker's job to update the docs).How about we (Mark?) create a spreadsheet, one sheet per class (?), list the current methods/overloads, note potential renames/etc for alignment with STL (soliciting comments), and have column(s) for documentation/examples and we crowd-source the docs?A willing volunteer would convert the results to Markdown when we're done.BTW, is this a comprehensive list of the WTF container types, or are there more lurking?BitVector Deque DoublyLinkedList HashCountedSet HashMap HashSet LinkedHashSet LinkedStack ListHashSet RefVector.h TerminatedArray(?) Vector
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAD649j6tYc-3MzksJaTQjb9n2eTHHXg7VgG179xT2_6dXc-ybA%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAFJcur9xpDWQrF5ZiG57s-Nb36gh4Fmi_RHbVHTH0FDOq-duKA%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAPG_qM7ZCE7wpw%3DBups0daLnun%2BF51aGVEdBBoR3mPTV80eFbw%40mail.gmail.com.
If so, I'd rather we bite the bullet and take the short-term pain to avoid the long-term lack of confusion.FWIW, having the names match where the behavior is the same makes sense to me too (roughly the list above lgtm). I agree with yutak's mental model for myself, but I find that a lot of people with less experience in WTF have a hard time quickly understanding code in third_party/WebKit partially because the names are different. I think it will help us feel more like one team if there are lower barriers between the different parts of the codebase.
There might be some I missed, but for me that would solve most of the problem. Of course we'd have two names for things. It would be possible to write a clang tool to rename everything, although that does seem to be quite a bit of work.On a side note I recently used WTF::HashMap with the key being a WTF::Vector<String> this was surprisingly difficult to write because it needed a custom hash traits class with lots of odd members. I note the STL also requires you to write a custom hash function and comparison for it's equivalent but that's very much easier to do. I don't have any specific proposal here, I appreciate the optimization that has gone into WTF::HashMap I just wish it was easier to write custom hash traits for it.
Thanks,Yuta
On Tue, Oct 25, 2016 at 3:15 AM, Joshua Bell <jsb...@chromium.org> wrote:
On Sun, Oct 23, 2016 at 11:16 PM, 'Aleks Totic' via platform-architecture-dev <platform-architecture-dev@chromium.org> wrote:I'd love to see better Oilpan integration docs inside .md, lots of valuable info is only available in not-so-readable template files. See https://crbug.com/655787 for examples. I volunteer for this if you need help.Regarding Oilpan, we have a tutorial and a full documentation.Full documentation has an empty section for Heap Collections.I've been unable to find docs about:- What collections are available, and what do they do (or just a pointer to standards class they are modeled after).- Do they accept WeakMembers? What are the performance characteristics?- Other details of using Oilpan, mentioned in the bug.After working on Chrome for a while, I've gained a lot of this knowledge from verbal communication, and reading of header files and existing code. It was an inefficient process.Having ~complete docs for foundational classes would help all new Chrome developers. In a perfect world, every time I have to ask a question about foundational stuff, it should be answered by a link to an updated doc (or make it asker's job to update the docs).How about we (Mark?) create a spreadsheet, one sheet per class (?), list the current methods/overloads, note potential renames/etc for alignment with STL (soliciting comments), and have column(s) for documentation/examples and we crowd-source the docs?A willing volunteer would convert the results to Markdown when we're done.BTW, is this a comprehensive list of the WTF container types, or are there more lurking?BitVector Deque DoublyLinkedList HashCountedSet HashMap HashSet LinkedHashSet LinkedStack ListHashSet RefVector.h TerminatedArray(?) Vector
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAD649j6tYc-3MzksJaTQjb9n2eTHHXg7VgG179xT2_6dXc-ybA%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAFJcur9xpDWQrF5ZiG57s-Nb36gh4Fmi_RHbVHTH0FDOq-duKA%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAPG_qM7ZCE7wpw%3DBups0daLnun%2BF51aGVEdBBoR3mPTV80eFbw%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CANMdWTsJLKY-vZCt%2BBoQhRqbK3SH_C%3DQ%3DFy3WcG7rqzhBaLB%2Bg%40mail.gmail.com.
I'd like to propose we initially add aliases for all WTF container methods that map 1:1 to stl functions e.g.WTF::Vector
- push_back = append
- erase = remove
- empty = isEmpty
- emplace_back = emplaceAppend
- pop_back = removeLast
- front = first
- back = last
WTF::HashMap
- insert = add
- erase = remove
- empty = isEmpty
- at = get
WTF::HashSet
- insert = add
- erase = remove
- empty = isEmpty
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/caa4b029-6444-486c-8318-6fab9aa01c9b%40chromium.org.
I've been looking at this and not all of the methods mentioned below have the same semantics as the stl. For example HashMap::remove() returns void, and the stl returns an iterator or a number (which for a Map is apparently always 1?).
As I said earlier in the thread I also think we should also leave isEmpty alone since is_empty and isEmpty are both common (and match our style guide), and also match the methods with the same name all over WTF and the resto of blink, and the stl naming here is confusing since it sounds like a verb (ex. clear() or reset()).
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/caa4b029-6444-486c-8318-6fab9aa01c9b%40chromium.org.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAO9Q3iL7Lq%2Bjd_pZVsfFJRxaPwj%2BAjdkACiYj%3DSu%3D-kAutHbjw%40mail.gmail.com.
On Tuesday, October 25, 2016 at 5:56:06 AM UTC-4, Alex Clarke wrote:I'd like to propose we initially add aliases for all WTF container methods that map 1:1 to stl functions e.g.WTF::Vector
- push_back = append
- erase = remove
- empty = isEmpty
- emplace_back = emplaceAppend
- pop_back = removeLast
- front = first
- back = last
Code Search estimates of method usage:append (2558)remove (108)isEmpty (623)emplaceAppend (13)removeLast (56)first (57+15) [T&+const T&]last (251+35) [T&+const T&]
2. check-webkit-style's readability/naming/underscores rule complains that emplace_back and pop_back are "incorrectly named. Don't use underscores in your identifier names." :)3. Please add yourself and/or other relevant people to the CLs for CC or review. (I will do this myself in future CLs, once I know whom to add.)-Mark
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/b6cf5417-6d92-42f6-8fb4-999a380e0e56%40chromium.org.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/139c350a-3fbf-4d12-9bcc-c7fa0633507b%40chromium.org.
1. There are a number of classes throughout Blink that essentially wrap WTF classes. For example, these 7 classes that act like vectors now define their own append() method that does nothing but call m_something.push_back():
~/work/src/third_party/WebKit/Source% grep -r "append(" * | grep "push_back("
core/xml/XPathNodeSet.h: void append(Node* node) { m_nodes.push_back(node); }
core/dom/DOMStringList.h: void append(const String& string) { m_strings.push_back(string); }
core/dom/TouchList.h: void append(Touch* touch) { m_values.push_back(touch); }
core/fileapi/FileList.h: void append(File* file) { m_files.push_back(file); }
core/css/CSSValueList.h: void append(const CSSValue& value) { m_values.push_back(value); }
platform/fonts/opentype/FontSettings.h: void append(const T& feature) { m_list.push_back(feature); }
wtf/RefVector.h: void append(const T& decoration) { m_vector.push_back(decoration); }
Also HTTPHeaderMap, which acts like a HashMap:
~/work/src/third_party/WebKit/Source% grep -r "remove(" * | grep "erase("
platform/network/HTTPHeaderMap.h: void remove(const AtomicString& k) { m_headers.erase(k); }
Question: do we want to extend the naming normalization to these wrapper classes?
2. I renamed methods in Vector but not Deque. Do we want to normalize Deque in the same way?
3. In order to rename HashSet::add() to ::insert(), I had to also an insert() to LinkedHashSet and ListHashSet, because there is testing code that assumes these classes have the same API. Do we want to normalize LinkedHashSet and ListHashSet in the same way?
4. In order to rename Vector::append() to ::push_back(), I had to completely change how HexNumber::appendByteAsHex() worked so we could essentially overload it with Vector-specific versions. [Thanks to jsbell for his technical assistance in actually implementing this, once we figured out what we wanted.] There is now one templated version of this function that calls destination.append() and two untemplated versions that take different kinds of Vectors and call destination.push_back(). This seems like a code smell, but I'm not sure what, if anything, to do about it.
5. Vector::emplace_back() now returns a reference to the new value that was appended, which matches the behavior of std::emplace_back. See https://codereview.chromium.org/2457163002 for discussion. This was the only semantic change so far.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/139c350a-3fbf-4d12-9bcc-c7fa0633507b%40chromium.org.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CADAgNurW4Tt%3D_zp4NSb6MT1VekzLdATT6dBj5jGwYVm426V-iQ%40mail.gmail.com.
I'm noticing that the Great Blink Rename has somewhat undone a bunch of pilgrim@'s hard work here to align WTF collection method names with STL. For example:WTF::Vector::data() is now WTF::Vector::Data() vs. std::vector::data()Capacity, Find, Clear, Swap, and a few others got uppercased.
It looks like some method names were kept (push/pop/back/front, r/begin/end, etc).
Do we want to undo that to continue aligning these with STL?
It does make the non-STL method names present on some collections (e.g. RemoveAll, IsEmpty) stand out more. It gets weird when there are not exact counterparts, e.g. std::vector has capacity() but std::set doesn't; should Capacity() be uppercase on WTF::HashSet ?
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAD649j6N8hVs%3DOoYHBLXjCWXU%2BEioYdZ17ngB%2Bv6PEJjs_b4fg%40mail.gmail.com.
On Wed, Apr 12, 2017 at 4:35 PM Joshua Bell <jsb...@chromium.org> wrote:I'm noticing that the Great Blink Rename has somewhat undone a bunch of pilgrim@'s hard work here to align WTF collection method names with STL. For example:WTF::Vector::data() is now WTF::Vector::Data() vs. std::vector::data()Capacity, Find, Clear, Swap, and a few others got uppercased.Yes, this is a known issue and tracked in https://bugs.chromium.org/p/chromium/issues/detail?id=709815.
It looks like some method names were kept (push/pop/back/front, r/begin/end, etc).Yeah, we missed a few in our list of exceptions to skip.Do we want to undo that to continue aligning these with STL?I think we should, hence the bug =)It does make the non-STL method names present on some collections (e.g. RemoveAll, IsEmpty) stand out more. It gets weird when there are not exact counterparts, e.g. std::vector has capacity() but std::set doesn't; should Capacity() be uppercase on WTF::HashSet ?I think it should be uppercase. Similar for the Find() overloads on HashSet/HashMap that take a HashTranslator, since they don't map 1:1 to an STL equivalent.
Daniel
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAD649j6N8hVs%3DOoYHBLXjCWXU%2BEioYdZ17ngB%2Bv6PEJjs_b4fg%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAF3XrKpBfyE6Ji2LW44%2B7M1bN1e74qJ8Vti1ThAZqeC15%2B61wQ%40mail.gmail.com.