General cleanup to remove friends and comply to this-> policy

This patches changes most opcode files to comply to the this-> policy we have to access member variables in C++.

It also removes all friend-statements from llvm_fbuilder.h

[ebo, 2010-07-27 07:37:10]

I need someone to review this patch, or give me a "commit for free"-card.

[Reid Kleckner, 2010-07-27 15:21:53]

Cool, thanks for working on this!

http://codereview.appspot.com/1847045/diff/1/24
File JIT/opcodes/attributes.cc (right):

http://codereview.appspot.com/1847045/diff/1/24#newcode92
JIT/opcodes/attributes.cc:92: llvm_data_(fbuilder->llvm_data())
It seems like for all of these opcode helper classes it would be good to have a
common base class to hold these fields and the BuilderT typedef.  The fields
would be made protected, and all these initialization lists would just call the
base constructor.

This would also help if/when we need to add fields to roughly all of the
opcodes.

http://codereview.appspot.com/1847045/diff/1/24#newcode205
JIT/opcodes/attributes.cc:205: BasicBlock *do_store =
this->state_->CreateBasicBlock("STORE_ATTR_do_store");
80 cols

http://codereview.appspot.com/1847045/diff/1/24#newcode345
JIT/opcodes/attributes.cc:345:
ConstantInt::get(PyTypeBuilder<long>::get(this->fbuilder_->context()),
We use the context all over for getting/creating types.  I think it would be
best to move it into the opcode base class I described above so we can replace
'this->fbuilder_->context()' with 'this->context_'.  The PyTypeBuilder lines are
the ones that tend to force us to line wrap.

http://codereview.appspot.com/1847045/diff/1/11
File JIT/opcodes/closure.cc (right):

http://codereview.appspot.com/1847045/diff/1/11#newcode66
JIT/opcodes/closure.cc:66:
PyTypeBuilder<Py_ssize_t>::get(this->fbuilder_->context()), num_defaults);
80 cols

http://codereview.appspot.com/1847045/diff/1/11#newcode68
JIT/opcodes/closure.cc:68: this->state_->GetGlobalFunction<PyObject
*(Py_ssize_t)>("PyTuple_New");
80 cols

http://codereview.appspot.com/1847045/diff/1/11#newcode135
JIT/opcodes/closure.cc:135:
ConstantInt::get(PyTypeBuilder<int>::get(this->fbuilder_->context()), index));
80 cols

[ebo, 2010-07-28 08:00:23]

http://codereview.appspot.com/1847045/diff/1/24
File JIT/opcodes/attributes.cc (right):

http://codereview.appspot.com/1847045/diff/1/24#newcode92
JIT/opcodes/attributes.cc:92: llvm_data_(fbuilder->llvm_data())
On 2010/07/27 15:21:53, Reid Kleckner wrote:
> It seems like for all of these opcode helper classes it would be good to have
a
> common base class to hold these fields and the BuilderT typedef.  The fields
> would be made protected, and all these initialization lists would just call
the
> base constructor.
> 
> This would also help if/when we need to add fields to roughly all of the
> opcodes.

Maybe I'll do that in a later patch. Problem is, that our coding guidelines do
not allow protected members, so I would have to rename everything again, to use
accessors. Ugh.

http://codereview.appspot.com/1847045/diff/1/24#newcode345
JIT/opcodes/attributes.cc:345:
ConstantInt::get(PyTypeBuilder<long>::get(this->fbuilder_->context()),
On 2010/07/27 15:21:53, Reid Kleckner wrote:
> We use the context all over for getting/creating types.  I think it would be
> best to move it into the opcode base class I described above so we can replace
> 'this->fbuilder_->context()' with 'this->context_'.  The PyTypeBuilder lines
are
> the ones that tend to force us to line wrap.

IMHO we don't use TypeBuilder that often ... If it is used more then twice you
can always create a local reference.

Design decision!

http://codereview.appspot.com/1847045/diff/1/11
File JIT/opcodes/closure.cc (right):

http://codereview.appspot.com/1847045/diff/1/11#newcode66
JIT/opcodes/closure.cc:66:
PyTypeBuilder<Py_ssize_t>::get(this->fbuilder_->context()), num_defaults);
On 2010/07/27 15:21:53, Reid Kleckner wrote:
> 80 cols

Done.

http://codereview.appspot.com/1847045/diff/1/11#newcode68
JIT/opcodes/closure.cc:68: this->state_->GetGlobalFunction<PyObject
*(Py_ssize_t)>("PyTuple_New");
On 2010/07/27 15:21:53, Reid Kleckner wrote:
> 80 cols

Done.

http://codereview.appspot.com/1847045/diff/1/11#newcode135
JIT/opcodes/closure.cc:135:
ConstantInt::get(PyTypeBuilder<int>::get(this->fbuilder_->context()), index));
On 2010/07/27 15:21:53, Reid Kleckner wrote:
> 80 cols

Done.

[Reid Kleckner (google), 2010-07-28 17:03:39]

Sure, LGTM.

Reid

On Wed, Jul 28, 2010 at 1:00 AM,  <ebo@4geeks.de> wrote:
>
> http://codereview.appspot.com/1847045/diff/1/24
> File JIT/opcodes/attributes.cc (right):
>
> http://codereview.appspot.com/1847045/diff/1/24#newcode92
> JIT/opcodes/attributes.cc:92: llvm_data_(fbuilder->llvm_data())
> On 2010/07/27 15:21:53, Reid Kleckner wrote:
>>
>> It seems like for all of these opcode helper classes it would be good
>
> to have a
>>
>> common base class to hold these fields and the BuilderT typedef.  The
>
> fields
>>
>> would be made protected, and all these initialization lists would just
>
> call the
>>
>> base constructor.
>
>> This would also help if/when we need to add fields to roughly all of
>
> the
>>
>> opcodes.
>
> Maybe I'll do that in a later patch. Problem is, that our coding
> guidelines do not allow protected members, so I would have to rename
> everything again, to use accessors. Ugh.
>
> http://codereview.appspot.com/1847045/diff/1/24#newcode345
> JIT/opcodes/attributes.cc:345:
> ConstantInt::get(PyTypeBuilder<long>::get(this->fbuilder_->context()),
> On 2010/07/27 15:21:53, Reid Kleckner wrote:
>>
>> We use the context all over for getting/creating types.  I think it
>
> would be
>>
>> best to move it into the opcode base class I described above so we can
>
> replace
>>
>> 'this->fbuilder_->context()' with 'this->context_'.  The PyTypeBuilder
>
> lines are
>>
>> the ones that tend to force us to line wrap.
>
> IMHO we don't use TypeBuilder that often ... If it is used more then
> twice you can always create a local reference.
>
> Design decision!
>
> http://codereview.appspot.com/1847045/diff/1/11
> File JIT/opcodes/closure.cc (right):
>
> http://codereview.appspot.com/1847045/diff/1/11#newcode66
> JIT/opcodes/closure.cc:66:
> PyTypeBuilder<Py_ssize_t>::get(this->fbuilder_->context()),
> num_defaults);
> On 2010/07/27 15:21:53, Reid Kleckner wrote:
>>
>> 80 cols
>
> Done.
>
> http://codereview.appspot.com/1847045/diff/1/11#newcode68
> JIT/opcodes/closure.cc:68: this->state_->GetGlobalFunction<PyObject
> *(Py_ssize_t)>("PyTuple_New");
> On 2010/07/27 15:21:53, Reid Kleckner wrote:
>>
>> 80 cols
>
> Done.
>
> http://codereview.appspot.com/1847045/diff/1/11#newcode135
> JIT/opcodes/closure.cc:135:
> ConstantInt::get(PyTypeBuilder<int>::get(this->fbuilder_->context()),
> index));
> On 2010/07/27 15:21:53, Reid Kleckner wrote:
>>
>> 80 cols
>
> Done.
>
> http://codereview.appspot.com/1847045/show
>