cc: Add PRESUBMIT check for passing simple gfx types by value.

cc: Add PRESUBMIT check for passing simple types by value.

This will save reviewer cycles, hooray.

R=enne
BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195719

[enne (OOO), 2013-04-22 18:40:22]

https://codereview.chromium.org/14410002/diff/4001/cc/PRESUBMIT.py
File cc/PRESUBMIT.py (right):

https://codereview.chromium.org/14410002/diff/4001/cc/PRESUBMIT.py#newcode66
cc/PRESUBMIT.py:66: white_list,
<_<
>_>

https://codereview.chromium.org/14410002/diff/4001/cc/PRESUBMIT.py#newcode106
cc/PRESUBMIT.py:106: r'\bconst +' + '(?P<type>gfx::(%s))&' %
I'm a little worried about false positives here.  What if somebody makes const
gfx::Vector2dF& on the stack for some reason?

[danakj, 2013-04-22 18:43:34]

https://codereview.chromium.org/14410002/diff/4001/cc/PRESUBMIT.py
File cc/PRESUBMIT.py (right):

https://codereview.chromium.org/14410002/diff/4001/cc/PRESUBMIT.py#newcode66
cc/PRESUBMIT.py:66: white_list,
On 2013/04/22 18:40:23, enne wrote:
> <_<
> >_>

Linter gets what linter wants. :p

https://codereview.chromium.org/14410002/diff/4001/cc/PRESUBMIT.py#newcode106
cc/PRESUBMIT.py:106: r'\bconst +' + '(?P<type>gfx::(%s))&' %
On 2013/04/22 18:40:23, enne wrote:
> I'm a little worried about false positives here.  What if somebody makes const
> gfx::Vector2dF& on the stack for some reason?

Hm.. true.. why must c++ be so hard to regex.

[danakj, 2013-04-22 20:57:30]

I think we don't want const refs to these types on the stack for the same
reasons we don't pass them by reference, or return them by reference. Added
base::Time/TimeTicks as well to the check.

[enne (OOO), 2013-04-22 22:04:09]

Ok, lgtm.

Before committing this, can you run this regex over cc/ and make sure there are
no false positives?  (Or, if there are true positives, maybe we should change
them in another CL.)

[danakj, 2013-04-23 01:46:52]

There's a lot of matches right now. I can do them in this CL or another,
whatever you like. I'll upload it in this one for now.

[commit-bot: I haz the power, 2013-04-23 02:14:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/14410002/16001

[commit-bot: I haz the power, 2013-04-23 05:16:42]

Change committed as 195719

[Tom Hudson, 2013-05-29 08:38:48]

Where did this come from? Gardening Blink I see changes being made in the
opposite direction (passing a 4-int class by const ref instead of by copy), and
I can't find anything about it in the google or chromium style guides. Also, it
gives me ARM performance worries.

[danakj, 2013-05-29 11:38:56]

See crbug.com/159273
On May 29, 2013 10:38 AM, <tomhudson@chromium.org> wrote:

> Where did this come from? Gardening Blink I see changes being made in the
> opposite direction (passing a 4-int class by const ref instead of by
> copy), and
> I can't find anything about it in the google or chromium style guides.
> Also, it
> gives me ARM performance worries.
>
>
https://codereview.chromium.**org/14410002/<https://codereview.chromium.org/1...
>