Issue 24823002: The scope of a function's signature is the function's enclosing scope, not the formal parameters sc…

Issue 24823002: The scope of a function's signature is the function's enclosing scope, not the formal parameters sc… (Closed)

Created:
7 years, 2 months ago by scheglov

Modified:
7 years, 2 months ago

Reviewers:
jwren, Brian Wilkerson

CC:
reviews_dartlang.org, lukeechurch_gmail.com

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

The scope of a function's signature is the function's enclosing scope, not the formal parameters scope. This is the recent change in spec. Implementation for it is not nice though... R=brianwilkerson@google.com, jwren@google.com BUG= Committed: https://code.google.com/p/dart/source/detail?r=28031

Patch Set 1 #

Total comments: 2

Created: 7 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+91 lines, -16 lines)			Patch
M	editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/resolver/ScopedVisitor.java	View	2 chunks	+14 lines, -0 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/scope/FunctionScope.java	View	2 chunks	+9 lines, -4 lines	2 comments	Download
M	editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/scope/FunctionTypeScope.java	View	4 chunks	+11 lines, -4 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/NonErrorResolverTest.java	View	3 chunks	+56 lines, -0 lines	0 comments	Download
M	editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/ResolverTestCase.java	View	1 chunk	+1 line, -1 line	0 comments	Download
M	tests/co19/co19-analyzer.status	View	2 chunks	+0 lines, -7 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

jwren

On 2013/09/26 20:25:17, scheglov wrote: LGTM Not pretty though like you commented.

7 years, 2 months ago (2013-09-26 20:33:17 UTC) #2

Brian Wilkerson

LGTM https://codereview.chromium.org/24823002/diff/1/editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/scope/FunctionScope.java File editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/scope/FunctionScope.java (right): https://codereview.chromium.org/24823002/diff/1/editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/scope/FunctionScope.java#newcode43 editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/scope/FunctionScope.java:43: if (parametersDefined) { Just out of curiosity, why ...

7 years, 2 months ago (2013-09-27 12:26:09 UTC) #3

scheglov

Committed patchset #1 manually as r28031 (presubmit successful).

7 years, 2 months ago (2013-09-27 23:37:31 UTC) #4

scheglov

7 years, 2 months ago (2013-09-27 23:40:07 UTC) #5

Message was sent while issue was closed.

https://codereview.chromium.org/24823002/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/scope/FunctionScope.java
(right):

https://codereview.chromium.org/24823002/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/scope/FunctionScope.java:43:
if (parametersDefined) {
On 2013/09/27 12:26:10, Brian Wilkerson wrote:
> Just out of curiosity, why are we invoking this method multiple times? (Looks
> like a problem that we should come back to later.)

It seems that ResolverVisitor redirects to two other visitors, so we visit
single node twice.

Expand Messages | Collapse Messages