Fix Intrinsifier and MethodRecognizer to correctly match private getters and setters.

Fix Intrinsifier and MethodRecognizer to correctly match private getters and setters.

R=fschneider@google.com

Committed: https://code.google.com/p/dart/source/detail?r=16701

[Florian Schneider, 2013-01-07 13:15:48]

LGTM with comments

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:220: static bool StartsWith(const String&
name, const char* prefix, intptr_t n) {
Maybe assert that name is a OneByteString?

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:238: static const intptr_t kPrefixLength =
5;
Do you really need static here? I'd omit it for local variables.

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:238: static const intptr_t kPrefixLength =
5;
Maybe rather use strlen(kPrefixGetter) and strlen(kPrefixSetter) below?

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:239: static const char* kGetterPrefix =
"get:_";
We have already kGetterPrefix in object.cc. Maybe define this one too in the
same place and rename it to kPrivateGetterPrefix.

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intrinsifier.cc
File runtime/vm/intrinsifier.cc (right):

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intrinsifier.cc#ne...
runtime/vm/intrinsifier.cc:18: static const char* kSetterPrefix = "set:_";
I'd define those in one place and remove kPrefixLength in favor of strlen(...).

[Kevin Millikin (Google), 2013-01-07 13:26:36]

DBC :)

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:238: static const intptr_t kPrefixLength =
5;
On 2013/01/07 13:15:48, Florian Schneider wrote:
> Maybe rather use strlen(kPrefixGetter) and strlen(kPrefixSetter) below? 

You can also use the trick:

static const char kGetterPrefix[] = "get:_";
static const char kPrefixLength = sizeof(kGetterPrefix);

[Kevin Millikin (Google), 2013-01-07 13:27:24]

On 2013/01/07 13:26:36, kmillikin wrote:
> DBC :)
> 
>
https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
> File runtime/vm/intermediate_language.cc (right):
> 
>
https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
> runtime/vm/intermediate_language.cc:238: static const intptr_t kPrefixLength =
> 5;
> On 2013/01/07 13:15:48, Florian Schneider wrote:
> > Maybe rather use strlen(kPrefixGetter) and strlen(kPrefixSetter) below? 
> 
> You can also use the trick:
> 
> static const char kGetterPrefix[] = "get:_";
> static const char kPrefixLength = sizeof(kGetterPrefix);

Make that sizeof(kGetterPrefix) - 1.  Duh.

[Vyacheslav Egorov (Google), 2013-01-07 15:02:47]

landed. thanks

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:220: static bool StartsWith(const String&
name, const char* prefix, intptr_t n) {
On 2013/01/07 13:15:48, Florian Schneider wrote:
> Maybe assert that name is a OneByteString?

Done.

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:238: static const intptr_t kPrefixLength =
5;
On 2013/01/07 13:15:48, Florian Schneider wrote:
> Do you really need static here? I'd omit it for local variables.

Done.

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:238: static const intptr_t kPrefixLength =
5;
On 2013/01/07 13:15:48, Florian Schneider wrote:
> Maybe rather use strlen(kPrefixGetter) and strlen(kPrefixSetter) below? 

Done.

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intermediate_langu...
runtime/vm/intermediate_language.cc:239: static const char* kGetterPrefix =
"get:_";
On 2013/01/07 13:15:48, Florian Schneider wrote:
> We have already kGetterPrefix in object.cc. Maybe define this one too in the
> same place and rename it to kPrivateGetterPrefix.

Done.

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intrinsifier.cc
File runtime/vm/intrinsifier.cc (right):

https://codereview.chromium.org/11779018/diff/1/runtime/vm/intrinsifier.cc#ne...
runtime/vm/intrinsifier.cc:18: static const char* kSetterPrefix = "set:_";
On 2013/01/07 13:15:48, Florian Schneider wrote:
> I'd define those in one place and remove kPrefixLength in favor of
strlen(...).

I renamed & started to use strlen. 

I am however going to keep it here. Later we will refactor method recognizer and
intrinsifier to use the same code.