Create separate scopes for constructors, setters, and other members.

Create separate scopes for constructors, setters, and other members.

R=karlklose@google.com

Committed: https://github.com/dart-lang/sdk/commit/b06e4d575450c47c48cecc4c9bdc096ff3698bd0

[ahe, 2017-03-31 15:45:54]

ahe@google.com changed reviewers:
+ karlklose@google.com

[karlklose, 2017-04-03 08:05:37]

LGTM

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart (right):

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:183: // A legal
name.
How about 'A legal name for a method, but not for a constructor name'?

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/scope.dart (right):

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/scope.dart:180: buildAmbiguousBuilder(String name,
Builder existing, Builder member)) {
Consider adding a typedef for this parameter's type to make the declaration
shorter.

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/source/source_library_builder.dart (right):

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/source/source_library_builder.dart:252:
(builder.isConstructor || builder.isFactory);
It feels wrong that 'isConstructor' has two different meanings here. Maybe the
flag on 'builder' should be called 'isGenerativeConstructor'?

[ahe, 2017-04-04 09:54:51]

Thank you, Karl!

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart (right):

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/kernel/kernel_library_builder.dart:183: // A legal
name.
On 2017/04/03 08:05:37, karlklose wrote:
> How about 'A legal name for a method, but not for a constructor name'?

Done.

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/scope.dart (right):

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/scope.dart:180: buildAmbiguousBuilder(String name,
Builder existing, Builder member)) {
On 2017/04/03 08:05:37, karlklose wrote:
> Consider adding a typedef for this parameter's type to make the declaration
> shorter.

I prefer to not introduce new typedefs unless the syntax forces me. Also, I find
this syntax easier to read than:

BuildAmbiguousBuilder buildAmbiguousBuilder

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
File pkg/front_end/lib/src/fasta/source/source_library_builder.dart (right):

https://codereview.chromium.org/2788153002/diff/20001/pkg/front_end/lib/src/f...
pkg/front_end/lib/src/fasta/source/source_library_builder.dart:252:
(builder.isConstructor || builder.isFactory);
On 2017/04/03 08:05:37, karlklose wrote:
> It feels wrong that 'isConstructor' has two different meanings here. Maybe the
> flag on 'builder' should be called 'isGenerativeConstructor'?

Perhaps, but on the other hand, kernel uses the name Constructor to represent
that concept. An alternative solution would be to come up with a good name for
the fields that are called "constructors" but contains factories and
constructors. Any ideas for a name for the concept "constructor or factory"?

[ahe, 2017-04-05 14:07:40]

Description was changed from

==========
Create separate scopes for constructors, setters, and other members.
==========

to

==========
Create separate scopes for constructors, setters, and other members.

R=karlklose@google.com

Committed:
https://github.com/dart-lang/sdk/commit/b06e4d575450c47c48cecc4c9bdc096ff3698bd0
==========

[ahe, 2017-04-05 14:07:41]

Committed patchset #5 (id:80001) manually as
b06e4d575450c47c48cecc4c9bdc096ff3698bd0 (presubmit successful).