check-webkit-style should warn about types we normally pass as reference If check-webkit-style ever sees a IntSize or IntPoint passed by value, it should warn. (I'm sure there are a bunch of others). I can't think of any reason to no pass them by const &. https://bugs.webkit.org/show_bug.cgi?id=60592#c6
Despite the fact that I think this is probably a good guideline, there are at least two possible benefits to passing a type like IntPoint by value instead of by reference. First, Anders Carlsson has claimed that in some situations on some platforms it may be more efficient. He can fill in the details at some point when he has the chance. Second, if you pass a point by reference, you can run into cases where reentrancy can change the value. Here's an example: scrollTo(m_documentSize) inside the scrollTo function FrameView::scrollTo(const IntPoint& position) { ... position has the value passed in ... ... do some work that changes the document size ... ... position is now the new document size ... } This can mean that if you, say, check for a special value such as zero, the value can change later in the same function.
I'm happy to close this as invalid then. Thank you for the background.
Oh, I wasn’t saying this is invalid. I think the rule is probably a good one. I was just responding to your comment that there are *no* reasons to ever prefer, say, IntPoint, to const IntPoint&. We might still decide that for our project these are never compelling reasons to do so, and perhaps should.
OK. Then sounds like we should come up with a list of types which we generally pass this way and add a rule to the style bot. :)
If passing an IntPoint by value would save a register on Mac OS X , then maybe we should just do that.
I guess my comment didn't make a lot of sense. It's just my feeling that passing by value would be better for performance on many platforms, but I'm not really an expert.
> If passing an IntPoint by value would save a register on Mac OS X We shouldn't forget 32-bit builds and ARM.
It looks like people are being guided to using "const IntPoint&" https://bugs.webkit.org/show_bug.cgi?id=60679#c5 Perhaps the guideline is if it begins with a capital letter (aka a custom defined type), then it is a pass by ref (const is optional) or pointer but not by value. Then if there are exceptions to this rule we can special case those types. To say that they should be pass by value, but it sounds like IntPoint is not an exception.
(In reply to comment #8) > It looks like people are being guided to using "const IntPoint&" > > https://bugs.webkit.org/show_bug.cgi?id=60679#c5 > > Perhaps the guideline is if it begins with a capital letter (aka a custom defined type), then it is a pass by ref (const is optional) or pointer but not by value. > > Then if there are exceptions to this rule we can special case those types. To say that they should be pass by value, but it sounds like IntPoint is not an exception. I'd just like to see a consensus so we can avoid butting heads on individual bugs when which is best could be argued.