Issue 10991034: Order the parameters of a function at the definition site.

Issue 10991034: Order the parameters of a function at the definition site. (Closed)

Created:
8 years, 2 months ago by ngeoffray

Modified:
8 years, 2 months ago

Reviewers:
kasperl

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Order the parameters of a function at the definition site. Committed: https://code.google.com/p/dart/source/detail?r=12904

Patch Set 3 : #

Created: 8 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+54 lines, -18 lines)			Patch
M	lib/compiler/implementation/compile_time_constants.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	lib/compiler/implementation/elements/elements.dart	View	1	2 chunks	+20 lines, -0 lines	0 comments	Download
M	lib/compiler/implementation/js_backend/emitter.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	lib/compiler/implementation/js_backend/namer.dart	View	1 2	1 chunk	+5 lines, -6 lines	0 comments	Download
M	lib/compiler/implementation/ssa/builder.dart	View	1	8 chunks	+8 lines, -8 lines	0 comments	Download
M	lib/compiler/implementation/ssa/codegen.dart	View	1	1 chunk	+4 lines, -1 line	0 comments	Download
M	lib/compiler/implementation/universe/universe.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
A	tests/language/named_argument_test.dart	View		1 chunk	+14 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

kasperl

LGTM. https://codereview.chromium.org/10991034/diff/1/lib/compiler/implementation/elements/elements.dart File lib/compiler/implementation/elements/elements.dart (right): https://codereview.chromium.org/10991034/diff/1/lib/compiler/implementation/elements/elements.dart#newcode978 lib/compiler/implementation/elements/elements.dart:978: optionalParameters.forEach((Element element) { list.add(element); }); Can you get ...

8 years, 2 months ago (2012-09-26 11:53:55 UTC) #2

ngeoffray

8 years, 2 months ago (2012-09-26 12:28:47 UTC) #3

Thanks Kasper

https://codereview.chromium.org/10991034/diff/1/lib/compiler/implementation/e...
File lib/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/10991034/diff/1/lib/compiler/implementation/e...
lib/compiler/implementation/elements/elements.dart:978:
optionalParameters.forEach((Element element) { list.add(element); });
On 2012/09/26 11:53:55, kasperl wrote:
> Can you get rid of this and use:
> 
>    list = new List<Element>.from(optionalParameters)

Good point. Done.

https://codereview.chromium.org/10991034/diff/1/lib/compiler/implementation/j...
File lib/compiler/implementation/js_backend/namer.dart (right):

https://codereview.chromium.org/10991034/diff/1/lib/compiler/implementation/j...
lib/compiler/implementation/js_backend/namer.dart:114: List<String>
optionalParameters = <String>[];
On 2012/09/26 11:53:55, kasperl wrote:
> Maybe check if optionalParameters.isEmpty first and just return methodName.
That
> should be a bit faster and it simplifies the logic a bit.

Done.

Expand Messages | Collapse Messages