fix #27631, get/set methods no longer shadow index operator

fix #27631, get/set methods no longer shadow index operator

R=rnystrom@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/511167581012ddb7610912f9f4aabf74e55d1ab2

[Jennifer Messerly, 2016-10-20 20:03:22]

jmesserly@google.com changed reviewers:
+ rnystrom@google.com

[kevmoo, 2016-10-20 20:06:02]

kevmoo@google.com changed reviewers:
+ kevmoo@google.com

[kevmoo, 2016-10-20 20:06:03]

DBC

So glad we're fixing this...

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:5287: if (name == '[]') {
better as a switch, no?

[Jennifer Messerly, 2016-10-20 20:12:15]

thanks!

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:5287: if (name == '[]') {
On 2016/10/20 20:06:03, kevmoo wrote:
> better as a switch, no?

I tend to find Dart's C-style switch clunky, e.g. if any condition needs to
change we'd have to edit it all back. Avoiding churn is a worthwhile goal as
well IMO. I don't think the Dart style team has much advice on this either way.

Personally, I hope we get pattern matching syntax in Dart.

Anyway happy to let Bob be the tiebreaker :)

[Jennifer Messerly, 2016-10-20 20:53:43]

jmesserly@google.com changed reviewers:
+ vsm@google.com

[Bob Nystrom, 2016-10-20 21:01:13]

LGTM!

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4485: if (operatorName ==
'-') operatorName = 'unary-';
Is "unary-" just used as a magic string for _emitMemberName()? Can you just use
"_negate" here instead?

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:5270: /// Unary minus
looks like: `x._negate()`. Note that [unary] must be passed
Can you update the part of the doc around "[unary]" since that parameter is gone
now?

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:5287: if (name == '[]') {
On 2016/10/20 20:12:14, Jennifer Messerly wrote:
> On 2016/10/20 20:06:03, kevmoo wrote:
> > better as a switch, no?
> 
> I tend to find Dart's C-style switch clunky, e.g. if any condition needs to
> change we'd have to edit it all back. Avoiding churn is a worthwhile goal as
> well IMO. I don't think the Dart style team has much advice on this either
way.
> 
> Personally, I hope we get pattern matching syntax in Dart.
> 
> Anyway happy to let Bob be the tiebreaker :)

If it was my code, I'd probably use switch () to highlight that each condition
is just doing a strict comparison with a string literal. But I also care less
about minimizing local code churn—I prefer a codebase to be an ideal
implementation of what it currently does and less so a speculation of what we
may want it to do in the future.

But I don't have a strong opinion either way, so I'll let you do what you like.

[Jennifer Messerly, 2016-10-20 22:45:04]

thanks

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4485: if (operatorName ==
'-') operatorName = 'unary-';
On 2016/10/20 21:01:13, Bob Nystrom wrote:
> Is "unary-" just used as a magic string for _emitMemberName()? Can you just
use
> "_negate" here instead?

it's not. It's defined by the Dart language spec and used by Analyzer.

https://codereview.chromium.org/2435163002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:5270: /// Unary minus
looks like: `x._negate()`. Note that [unary] must be passed
On 2016/10/20 21:01:13, Bob Nystrom wrote:
> Can you update the part of the doc around "[unary]" since that parameter is
gone
> now?

Done.

[Jennifer Messerly, 2016-10-20 22:45:36]

Description was changed from

==========
fix #27631, get/set methods no longer shadow index operator
==========

to

==========
fix #27631, get/set methods no longer shadow index operator

R=rnystrom@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/511167581012ddb7610912f9f4aa...
==========

[Jennifer Messerly, 2016-10-20 22:45:38]

Committed patchset #2 (id:10001) manually as
511167581012ddb7610912f9f4aabf74e55d1ab2 (presubmit successful).