Issue 9109015: AST nodes for function-typed parameters.

Issue 9109015: AST nodes for function-typed parameters. (Closed)

Created:
8 years, 11 months ago by ahe

Modified:
8 years, 11 months ago

Reviewers:
karlklose, ngeoffray, kasperl, Lasse Reichstein Nielsen

CC:
reviews_dartlang.org, kasperl, floitsch

Base URL:
ssh://aaricia.aar/home/ahe/Dart/all/dart.googlecode.com.git@master

Visibility:
Public.

More Reviews

Description

AST nodes for function-typed parameters. Committed: https://code.google.com/p/dart/source/detail?r=3031

Patch Set 2 : Address review comments #

Patch Set 3 : rebased #

Created: 8 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+13 lines, -9 lines)			Patch
M	dart/frog/leg/resolver.dart	View	1 2	1 chunk	+0 lines, -2 lines	0 comments	Download
M	dart/frog/leg/scanner/listener.dart	View	1	1 chunk	+6 lines, -3 lines	0 comments	Download
M	dart/frog/leg/tree/nodes.dart	View		2 chunks	+5 lines, -2 lines	0 comments	Download
M	dart/frog/leg/tree/unparser.dart	View		1 chunk	+1 line, -1 line	0 comments	Download
M	dart/tests/language/language-leg.status	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

kasperl

DBC: http://codereview.chromium.org/9109015/diff/1/dart/tests/language/language-leg.status File dart/tests/language/language-leg.status (right): http://codereview.chromium.org/9109015/diff/1/dart/tests/language/language-leg.status#newcode46 dart/tests/language/language-leg.status:46: ClosuresWithComplexParamsTest: Fail Remove the Fail line?

8 years, 11 months ago (2012-01-06 07:52:46 UTC) #2

ngeoffray

LGTM http://codereview.chromium.org/9109015/diff/1/dart/frog/leg/scanner/listener.dart File dart/frog/leg/scanner/listener.dart (right): http://codereview.chromium.org/9109015/diff/1/dart/frog/leg/scanner/listener.dart#newcode1062 dart/frog/leg/scanner/listener.dart:1062: TypeAnnotation type = popNode(); Rename 'type' to 'returnType'? ...

8 years, 11 months ago (2012-01-06 08:20:46 UTC) #3

karlklose

LGTM. http://codereview.chromium.org/9109015/diff/1/dart/frog/leg/scanner/listener.dart File dart/frog/leg/scanner/listener.dart (right): http://codereview.chromium.org/9109015/diff/1/dart/frog/leg/scanner/listener.dart#newcode1063 dart/frog/leg/scanner/listener.dart:1063: pushNode(null); // Signal "no return type" to endFormalParameter. ...

8 years, 11 months ago (2012-01-06 08:22:59 UTC) #4

ahe

8 years, 11 months ago (2012-01-06 11:00:03 UTC) #5

Thanks for all you comments!

Cheers,
Peter

http://codereview.chromium.org/9109015/diff/1/dart/frog/leg/scanner/listener....
File dart/frog/leg/scanner/listener.dart (right):

http://codereview.chromium.org/9109015/diff/1/dart/frog/leg/scanner/listener....
dart/frog/leg/scanner/listener.dart:1062: TypeAnnotation type = popNode();
On 2012/01/06 08:20:46, ngeoffray wrote:
> Rename 'type' to 'returnType'?

Done.

http://codereview.chromium.org/9109015/diff/1/dart/frog/leg/scanner/listener....
dart/frog/leg/scanner/listener.dart:1063: pushNode(null); // Signal "no return
type" to endFormalParameter.
On 2012/01/06 08:23:00, karlklose wrote:
> I think it should be "no type".

Done.

http://codereview.chromium.org/9109015/diff/1/dart/frog/leg/tree/unparser.dart
File dart/frog/leg/tree/unparser.dart (right):

http://codereview.chromium.org/9109015/diff/1/dart/frog/leg/tree/unparser.dar...
dart/frog/leg/tree/unparser.dart:155: if (node.endToken !== null)
add(node.endToken.value);
On 2012/01/06 08:20:46, ngeoffray wrote:
> Isn't the endToken the semi-colon? How can it be null?

For a function expression:

print((() => 42)());

http://codereview.chromium.org/9109015/diff/1/dart/tests/language/language-le...
File dart/tests/language/language-leg.status (right):

http://codereview.chromium.org/9109015/diff/1/dart/tests/language/language-le...
dart/tests/language/language-leg.status:46: ClosuresWithComplexParamsTest: Fail
On 2012/01/06 07:52:46, kasperl wrote:
> Remove the Fail line?

Good catch. Good thing I sorted the lines before uploading the CL :-)

Expand Messages | Collapse Messages