Subzero: Class definition cleanup.

Subzero: Class definition cleanup.

For consistency, put deleted ctors at the beginning of the class
definition.

If the default copy ctor or assignment operator is not deleted,
and the default implementation is used, leave it commented out to
indicate it is intentional.

Also, fixed one C++11 related TODO.

BUG= none
R=jvoung@chromium.org, kschimpf@google.com

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=7b451a928d91d1c38805c7a53e18519000fe7c42

[Jim Stichnoth, 2014-10-15 19:27:50]

stichnot@chromium.org changed reviewers:
+ jvoung@chromium.org, kschimpf@google.com

[Karl, 2014-10-15 19:59:13]

LGTM otherwise.

https://codereview.chromium.org/656123003/diff/1/src/IceInst.h
File src/IceInst.h (right):

https://codereview.chromium.org/656123003/diff/1/src/IceInst.h#newcode183
src/IceInst.h:183: : Inst(Func, Kind, MaxSrcs, Dest) {}
Is there a reason this doesn't have a create method (like other instructions).

https://codereview.chromium.org/656123003/diff/1/src/IceInstX8632.h
File src/IceInstX8632.h (right):

https://codereview.chromium.org/656123003/diff/1/src/IceInstX8632.h#newcode149
src/IceInstX8632.h:149: class SpillVariable : public Variable {
What about delete for default constructor/assignment.

https://codereview.chromium.org/656123003/diff/1/src/IceOperand.h
File src/IceOperand.h (right):

https://codereview.chromium.org/656123003/diff/1/src/IceOperand.h#newcode184
src/IceOperand.h:184: RelocatableTuple &operator=(const RelocatableTuple &) =
delete;
What about the default constructor (or at least a commented out version)?

[Jim Stichnoth, 2014-10-15 20:23:44]

https://codereview.chromium.org/656123003/diff/1/src/IceInst.h
File src/IceInst.h (right):

https://codereview.chromium.org/656123003/diff/1/src/IceInst.h#newcode183
src/IceInst.h:183: : Inst(Func, Kind, MaxSrcs, Dest) {}
On 2014/10/15 19:59:12, Karl wrote:
> Is there a reason this doesn't have a create method (like other instructions).

This is the base class for the high-level ICE instructions, and isn't meant to
be constructed directly.  Similar for InstTarget at the end of the file.

https://codereview.chromium.org/656123003/diff/1/src/IceInstX8632.h
File src/IceInstX8632.h (right):

https://codereview.chromium.org/656123003/diff/1/src/IceInstX8632.h#newcode149
src/IceInstX8632.h:149: class SpillVariable : public Variable {
On 2014/10/15 19:59:12, Karl wrote:
> What about delete for default constructor/assignment.

Done.

https://codereview.chromium.org/656123003/diff/1/src/IceOperand.h
File src/IceOperand.h (right):

https://codereview.chromium.org/656123003/diff/1/src/IceOperand.h#newcode184
src/IceOperand.h:184: RelocatableTuple &operator=(const RelocatableTuple &) =
delete;
On 2014/10/15 19:59:12, Karl wrote:
> What about the default constructor (or at least a commented out version)?

It is actually explicitly defined in the public: section, but in a way that is
no different than leaving the default implementation, so I changed it
accordingly.

[Karl, 2014-10-15 20:29:12]

lgtm

https://codereview.chromium.org/656123003/diff/1/src/IceInst.h
File src/IceInst.h (right):

https://codereview.chromium.org/656123003/diff/1/src/IceInst.h#newcode183
src/IceInst.h:183: : Inst(Func, Kind, MaxSrcs, Dest) {}
On 2014/10/15 20:23:43, stichnot wrote:
> On 2014/10/15 19:59:12, Karl wrote:
> > Is there a reason this doesn't have a create method (like other
instructions).
> 
> This is the base class for the high-level ICE instructions, and isn't meant to
> be constructed directly.  Similar for InstTarget at the end of the file.

I see. I didn't notice it was protected, and hence can only be built in derived
classes.

[jvoung (off chromium), 2014-10-15 21:23:38]

LGTM too -- there are a couple of "delete" that can be moved up in the assembler
files too, but I could do those later if you want.

[Jim Stichnoth, 2014-10-15 21:38:57]

On 2014/10/15 21:23:38, jvoung wrote:
> LGTM too -- there are a couple of "delete" that can be moved up in the
assembler
> files too, but I could do those later if you want.

Ah right, I should have clarified that I didn't want to mess with files I knew
you were actively working on. :)

[Jim Stichnoth, 2014-10-15 21:39:42]

Committed patchset #2 (id:20001) manually as
7b451a928d91d1c38805c7a53e18519000fe7c42 (presubmit successful).