styleguide: Move Rvalue References to the Blacklist with Exceptions.

styleguide: Move Rvalue References to the Blacklist with Exceptions.

The discussion around Rvalue references has been that we should ban
their use for now, but come back to it later. So that is what this
does.

One exceptional case that was agreed upon without dissent was to allow
move semantics in scoped_ptr to allow use of scoped_ptr cleaner
throughout the codebase, without leaking the requirement to understand
Rvalue references outside of the scoped_ptr implementation.

R=jamesr@chromium.org, thakis@chromium.org

Committed: https://crrev.com/d9a02e76127634286ff0c760f87db260b367492a
Cr-Commit-Position: refs/heads/master@{#297016}

[danakj, 2014-09-26 01:29:25]

danakj@chromium.org changed reviewers:
+ ajwong@chromium.org

[danakj, 2014-09-26 01:29:25]

+ajwong in case he has input if making an explicit whitelist here is a good
idea, or just leave it in the blacklist but land things with it anyways where
chromium-dev seems to think it's okay

[awong, 2014-09-26 02:58:37]

On 2014/09/26 01:29:25, danakj wrote:
> +ajwong in case he has input if making an explicit whitelist here is a good
> idea, or just leave it in the blacklist but land things with it anyways where
> chromium-dev seems to think it's okay

I think a whitelist might be appropriate for now. There are a few very specific
places where we should use these...such as Bind().

[jamesr, 2014-09-26 04:40:54]

My feeling is encoding a whitelist encoded here is too bureaucratic.  I'd rather
put something here like "if you need to use a feature on the banned or to be
discussed list, do XYZ" where hopefully XYZ is really simple and straightforwad.
Maybe just "add `cat styleguide/c++/OWNERS` to the review list and proceed when
you get an lgtm" ?  I don't think we'll have so many cases of these that we need
a list in source control of exceptions to keep track of it all, or at least I
hope we don't.  If we do we can revisit later.

[danakj, 2014-09-26 13:29:05]

Changed to "To be revisited in the future.  Allowed in exceptional cases where
approved on chromium-dev." Or still too perscriptive?

[brettw, 2014-09-26 15:21:41]

Asking chromium-dev is a recipe for a neverending centithread. I liked James'
suggestion better

[danakj, 2014-09-26 15:39:29]

On 2014/09/26 15:21:41, brettw wrote:
> Asking chromium-dev is a recipe for a neverending centithread. I liked James'
> suggestion better

Ya, I guess so. It seemed more open tho. But probably hellishly unproductive.

"To be revisited in the future.  Allowed in exceptional cases where approved by
the OWNERS of src/styleguide/c++/."

[brettw, 2014-09-26 18:16:23]

On 2014/09/26 15:39:29, danakj wrote:
> On 2014/09/26 15:21:41, brettw wrote:
> > Asking chromium-dev is a recipe for a neverending centithread. I liked
James'
> > suggestion better
> 
> Ya, I guess so. It seemed more open tho. But probably hellishly unproductive.
> 
> "To be revisited in the future.  Allowed in exceptional cases where approved
by
> the OWNERS of src/styleguide/c++/."

sounds good to me.

[jamesr, 2014-09-26 18:22:13]

lgtm, but give awong a chance to chime in too before landing please.  Thanks!

[awong, 2014-09-26 18:29:19]

LGTM

[danakj, 2014-09-26 19:24:31]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-09-26 19:25:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/607773002/40001

[commit-bot: I haz the power, 2014-09-26 20:03:12]

Committed patchset #3 (id:40001) as e26abb63908b75a37554c705841bd8008b01dc34

[commit-bot: I haz the power, 2014-09-26 20:03:52]

Patchset 3 (id:??) landed as
https://crrev.com/d9a02e76127634286ff0c760f87db260b367492a
Cr-Commit-Position: refs/heads/master@{#297016}

[Avi (use Gerrit), 2014-09-26 20:33:23]

avi@chromium.org changed reviewers:
+ avi@chromium.org

[Avi (use Gerrit), 2014-09-26 20:33:23]

https://codereview.chromium.org/607773002/diff/40001/styleguide/c++/c++11.html
File styleguide/c++/c++11.html (right):

https://codereview.chromium.org/607773002/diff/40001/styleguide/c++/c++11.htm...
styleguide/c++/c++11.html:186: </table>
Is there a reason you put this in a different table than all the other features
that were banned?

[danakj, 2014-09-26 20:41:53]

It's header says "<p>This section lists features that are not allowed to be used
yet." which is what the Note also says.

Probably some of the others should move. I think that table is meant for more
final decisions?

[Avi (use Gerrit), 2014-09-26 20:56:07]

On 2014/09/26 20:41:53, danakj wrote:
> It's header says "<p>This section lists features that are not allowed to be
used
> yet." which is what the Note also says.
> 
> Probably some of the others should move. I think that table is meant for more
> final decisions?

Look at the header levels on the previous version of this file.

h2 is the banned section, which covers both the h3 banned feature table and the
h3 standard library. You put a table directly underneath the h2 header.

If there is a slight difference between "banned but ok if specifically approved"
and "always banned", then at least put an h2 header on the table.

(Or I can do it if you wish.)

[Avi (use Gerrit), 2014-09-26 20:57:29]

On 2014/09/26 20:56:07, Avi wrote:
> If there is a slight difference between "banned but ok if specifically
approved"
> and "always banned", then at least put an h2 header on the table.

At least put an h3 header on the file...

[danakj, 2014-09-26 20:59:41]

On Fri, Sep 26, 2014 at 4:57 PM, <avi@chromium.org> wrote:

> On 2014/09/26 20:56:07, Avi wrote:
>
>> If there is a slight difference between "banned but ok if specifically
>>
> approved"
>
>> and "always banned", then at least put an h2 header on the table.
>>
>
> At least put an h3 header on the file...
>

Ohhh, I misread the HTML clearly. OK let me try patch this up. Thanks.


>
> https://codereview.chromium.org/607773002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.