Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(732)

Issue 2059883002: Implementation of modified scoping for initializing formals. (Closed)

Created:
4 years, 6 months ago by eernst
Modified:
4 years, 5 months ago
Reviewers:
Johnni Winther
CC:
reviews_dartlang.org, floitsch
Base URL:
https://github.com/dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Implementation of modified scoping for initializing formals. In order to make an implementation of initializing formal access in `dart2js` available as specified in issue #26655 now, this CL changes the scope management such that initializing formals are not in scope in the constructor body, only in the initializer list. This is done by introducing a new notion of scopes implemented by `ExtensionScope`: Such a scope will extend an existing `NestedScope` rather than adding a new nested scope to it. R=johnniwinther@google.com Committed: https://github.com/dart-lang/sdk/commit/442fc53b3c2d1baa4ac8b207e13f3c859959229e

Patch Set 1 #

Total comments: 2

Patch Set 2 : Review response #

Patch Set 3 : Response to IRL discussion #

Total comments: 4

Patch Set 4 : Review response #

Unified diffs Side-by-side diffs Delta from patch set Stats (+120 lines, -27 lines) Patch
M pkg/compiler/lib/src/diagnostics/messages.dart View 1 2 3 1 chunk +20 lines, -0 lines 0 comments Download
M pkg/compiler/lib/src/resolution/constructors.dart View 1 2 5 chunks +26 lines, -1 line 0 comments Download
M pkg/compiler/lib/src/resolution/members.dart View 1 2 1 chunk +0 lines, -5 lines 0 comments Download
M pkg/compiler/lib/src/resolution/resolution.dart View 1 2 3 chunks +5 lines, -2 lines 0 comments Download
M pkg/compiler/lib/src/resolution/scope.dart View 1 2 3 2 chunks +45 lines, -2 lines 0 comments Download
M tests/compiler/dart2js/message_kind_helper.dart View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
A + tests/language/initializing_formal_scope_test.dart View 1 2 3 1 chunk +10 lines, -4 lines 0 comments Download
M tests/language/language.status View 1 2 chunks +8 lines, -8 lines 0 comments Download
M tests/language/language_analyzer2.status View 1 chunk +5 lines, -5 lines 0 comments Download

Messages

Total messages: 13 (5 generated)
eernst
Adjustment of scope rules for initializing formals.
4 years, 6 months ago (2016-06-10 18:06:14 UTC) #2
Johnni Winther
https://codereview.chromium.org/2059883002/diff/1/pkg/compiler/lib/src/resolution/resolution.dart File pkg/compiler/lib/src/resolution/resolution.dart (right): https://codereview.chromium.org/2059883002/diff/1/pkg/compiler/lib/src/resolution/resolution.dart#newcode252 pkg/compiler/lib/src/resolution/resolution.dart:252: if (compiler.options.enableInitializingFormalAccess) { There is a better approach: 1. ...
4 years, 6 months ago (2016-06-13 08:56:48 UTC) #4
eernst
Didn't finish, but I will be OOO for 7 work days now, so I've just ...
4 years, 6 months ago (2016-06-14 15:01:51 UTC) #5
eernst
Review response: Now repeating the work done in `setupFunction` to get hold of the `parameterNode` ...
4 years, 5 months ago (2016-06-28 08:16:42 UTC) #6
eernst
Introduced a new `ExtensionScope` which allows for defining a scope whose bindings are the union ...
4 years, 5 months ago (2016-06-28 14:51:27 UTC) #7
Johnni Winther
lgtm https://codereview.chromium.org/2059883002/diff/40001/pkg/compiler/lib/src/resolution/scope.dart File pkg/compiler/lib/src/resolution/scope.dart (right): https://codereview.chromium.org/2059883002/diff/40001/pkg/compiler/lib/src/resolution/scope.dart#newcode224 pkg/compiler/lib/src/resolution/scope.dart:224: bool isDuplicate(Element element) => element == localLookup(element.name); Remove ...
4 years, 5 months ago (2016-06-29 07:22:24 UTC) #8
eernst
Review + IRL discussion response. https://codereview.chromium.org/2059883002/diff/40001/pkg/compiler/lib/src/resolution/scope.dart File pkg/compiler/lib/src/resolution/scope.dart (right): https://codereview.chromium.org/2059883002/diff/40001/pkg/compiler/lib/src/resolution/scope.dart#newcode224 pkg/compiler/lib/src/resolution/scope.dart:224: bool isDuplicate(Element element) => ...
4 years, 5 months ago (2016-06-29 09:28:32 UTC) #9
eernst
4 years, 5 months ago (2016-06-29 09:42:20 UTC) #13
Message was sent while issue was closed.
Committed patchset #4 (id:60001) manually as
442fc53b3c2d1baa4ac8b207e13f3c859959229e (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698