Mojo cpp bindings: support Clone() for structs and arrays which don't contain handles.

Mojo cpp bindings: support Clone() for structs and arrays which don't contain handles.

With this CL you can do:
  // It is safe to call even if foo.is_null() is true.
  FooStructPtr foo_clone = foo.Clone();
  Array<Bar> bar_array_clone = bar_array.Clone();

If you call Clone() with a struct or array type which (transitively) contains handles, you will get a compile-time error:
  'Clone' : is not a member of 'FooStruct'.

BUG=None
TEST=Newly added tests in array_unittest and struct_unittest

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

[yzshen1, 2014-10-02 00:04:15]

yzshen@chromium.org changed reviewers:
+ darin@chromium.org, sky@chromium.org

[yzshen1, 2014-10-02 00:04:15]

Hi, Darin and Scott.

Would you please take a look? Thanks!

[sky, 2014-10-02 17:14:20]

LGTM

[yzshen1, 2014-10-02 19:27:18]

The CQ bit was checked by yzshen@chromium.org

[commit-bot: I haz the power, 2014-10-02 19:28:11]

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

[darin (slow to review), 2014-10-02 20:14:14]

https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
File
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
(right):

https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl:25:
{{struct.name}}Ptr Clone() const;
Doesn't this potentially conflict with a user defined "Clone" function? Suppose
someone puts a method named "Clone" in their mojom that takes no parameters.

[yzshen1, 2014-10-02 20:18:31]

The CQ bit was unchecked by yzshen@chromium.org

[yzshen1, 2014-10-02 20:20:29]

On 2014/10/02 20:14:14, darin wrote:
>
https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
> File
>
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
> (right):
> 
>
https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
>
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl:25:
> {{struct.name}}Ptr Clone() const;
> Doesn't this potentially conflict with a user defined "Clone" function?
Suppose
> someone puts a method named "Clone" in their mojom that takes no parameters.

This Clone() is defined on the struct wrapper types, instead of the interface
types.
There isn't way for users to define functions for mojom structs. So I think it
won't be a problem.

What do you think? Thanks!

[yzshen1, 2014-10-02 20:21:12]

(I have stopped the commit queue. Sorry I thought you probably wouldn't review
this CL. :))

[darin (slow to review), 2014-10-02 20:25:30]

On 2014/10/02 20:21:12, yzshen1 wrote:
> (I have stopped the commit queue. Sorry I thought you probably wouldn't review
> this CL. :))

Oh, nevermind. This is a method on a struct not an interface. D'oh! Please
ignore me.

[yzshen1, 2014-10-02 20:29:30]

On 2014/10/02 20:25:30, darin wrote:
> On 2014/10/02 20:21:12, yzshen1 wrote:
> > (I have stopped the commit queue. Sorry I thought you probably wouldn't
review
> > this CL. :))
> 
> Oh, nevermind. This is a method on a struct not an interface. D'oh! Please
> ignore me.

Thanks Darin!
Then I will land the CL as it is.

[yzshen1, 2014-10-02 20:29:34]

The CQ bit was checked by yzshen@chromium.org

[commit-bot: I haz the power, 2014-10-02 20:31:02]

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

[commit-bot: I haz the power, 2014-10-02 20:32:21]

Committed patchset #3 (id:40001) as 05a56fffe533e9466a2073b0eaf100f87486c3b1

[commit-bot: I haz the power, 2014-10-02 20:33:04]

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

[Aaron Boodman, 2014-10-10 20:27:46]

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

[Aaron Boodman, 2014-10-10 20:27:47]

https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
File
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
(right):

https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl:25:
{{struct.name}}Ptr Clone() const;
On 2014/10/02 20:14:13, darin wrote:
> Doesn't this potentially conflict with a user defined "Clone" function?
Suppose
> someone puts a method named "Clone" in their mojom that takes no parameters.

It seems like it would. Since I am going to add Equals, was this question ever
resolved?

It seems like an alternate design would be to put these features on a generated
traits class, or as static members of {{struct.name}} class, or something.

[yzshen1, 2014-10-10 20:30:22]

https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
File
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
(right):

https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl:25:
{{struct.name}}Ptr Clone() const;
This question has been resolved.

Clone() is defined on struct wrapper types, instead of interface types.
There isn't way for users to define functions for mojom structs. So I think it
won't be a problem.

[Aaron Boodman, 2014-10-10 21:04:15]

https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
File
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
(right):

https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl:25:
{{struct.name}}Ptr Clone() const;
per our IM chat, this isn't resolved -- it is possible for user-defined fields
to conflict with these methods.

Users can't use the field name 'Clone'. In fact, this problem already existed --
Users can't use the field names 'New' or 'From' either.

The way I see it we can either just declare that mojom fields are always
unix_hacker_style, and so names starting with a capital are an open namespace
for bindings systems to use. Or else we can move these bindings-related methods
to some other traits class.

Darin, do you have an opinion?

[darin (slow to review), 2014-10-12 17:57:30]

https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
File
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
(right):

https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl:25:
{{struct.name}}Ptr Clone() const;
On 2014/10/10 21:04:15, Aaron Boodman wrote:
> per our IM chat, this isn't resolved -- it is possible for user-defined fields
> to conflict with these methods.
> 
> Users can't use the field name 'Clone'. In fact, this problem already existed
--
> Users can't use the field names 'New' or 'From' either.
> 
> The way I see it we can either just declare that mojom fields are always
> unix_hacker_style, and so names starting with a capital are an open namespace
> for bindings systems to use. Or else we can move these bindings-related
methods
> to some other traits class.
> 
> Darin, do you have an opinion?

Right, good catch. I'd be OK dictating that field names must start with a
lowercase letter. That matches every convention I know of, from
unix_hacker_style to camelCase, so it shouldn't make anyone sad.

Moving things to a traits class would work fine for Clone, but not for New and
From.

[Aaron Boodman, 2014-10-12 18:01:59]

On 2014/10/12 17:57:30, darin wrote:
>
https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
> File
>
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
> (right):
> 
>
https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
>
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl:25:
> {{struct.name}}Ptr Clone() const;
> On 2014/10/10 21:04:15, Aaron Boodman wrote:
> > per our IM chat, this isn't resolved -- it is possible for user-defined
fields
> > to conflict with these methods.
> > 
> > Users can't use the field name 'Clone'. In fact, this problem already
existed
> --
> > Users can't use the field names 'New' or 'From' either.
> > 
> > The way I see it we can either just declare that mojom fields are always
> > unix_hacker_style, and so names starting with a capital are an open
namespace
> > for bindings systems to use. Or else we can move these bindings-related
> methods
> > to some other traits class.
> > 
> > Darin, do you have an opinion?
> 
> Right, good catch. I'd be OK dictating that field names must start with a
> lowercase letter. That matches every convention I know of, from
> unix_hacker_style to camelCase, so it shouldn't make anyone sad.

ok.

> Moving things to a traits class would work fine for Clone, but not for New and
> From.

OOC, why would it not work for New and From. They are static methods already...

[Aaron Boodman, 2014-10-12 18:03:19]

On 2014/10/12 18:01:59, Aaron Boodman wrote:
> On 2014/10/12 17:57:30, darin wrote:
> >
>
https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
> > File
> >
>
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl
> > (right):
> > 
> >
>
https://codereview.chromium.org/615063003/diff/40001/mojo/public/tools/bindin...
> >
>
mojo/public/tools/bindings/generators/cpp_templates/wrapper_class_declaration.tmpl:25:
> > {{struct.name}}Ptr Clone() const;
> > On 2014/10/10 21:04:15, Aaron Boodman wrote:
> > > per our IM chat, this isn't resolved -- it is possible for user-defined
> fields
> > > to conflict with these methods.
> > > 
> > > Users can't use the field name 'Clone'. In fact, this problem already
> existed
> > --
> > > Users can't use the field names 'New' or 'From' either.
> > > 
> > > The way I see it we can either just declare that mojom fields are always
> > > unix_hacker_style, and so names starting with a capital are an open
> namespace
> > > for bindings systems to use. Or else we can move these bindings-related
> > methods
> > > to some other traits class.
> > > 
> > > Darin, do you have an opinion?
> > 
> > Right, good catch. I'd be OK dictating that field names must start with a
> > lowercase letter. That matches every convention I know of, from
> > unix_hacker_style to camelCase, so it shouldn't make anyone sad.
> 
> ok.

In that case, it would be good to enforce this in the SDK or, even better, at
runtime -- because otherwise there will eventually be an accidental collision.

> > Moving things to a traits class would work fine for Clone, but not for New
and
> > From.
> 
> OOC, why would it not work for New and From. They are static methods
already...