Issue 9634008: Introduce element categories and move resolver towards being more compositional.

Issue 9634008: Introduce element categories and move resolver towards being more compositional. (Closed)

Created:
8 years, 9 months ago by ahe

Modified:
8 years, 9 months ago

Reviewers:
karlklose, ngeoffray

CC:
reviews_dartlang.org, compiler-dev_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Introduce element categories and move resolver towards being more compositional. This also improves handling of super. Committed: https://code.google.com/p/dart/source/detail?r=5184

Patch Set 2 : Address review comments #

Created: 8 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+275 lines, -163 lines)			Patch
M	dart/frog/leg/elements/elements.dart	View	1	5 chunks	+82 lines, -30 lines	0 comments	Download
M	dart/frog/leg/resolver.dart	View	1	9 chunks	+73 lines, -56 lines	0 comments	Download
M	dart/frog/leg/scanner/listener.dart	View		1 chunk	+3 lines, -1 line	0 comments	Download
M	dart/frog/leg/scanner/parser.dart	View		2 chunks	+1 line, -27 lines	0 comments	Download
M	dart/frog/leg/scanner/token.dart	View		1 chunk	+25 lines, -0 lines	0 comments	Download
M	dart/frog/leg/ssa/builder.dart	View		2 chunks	+7 lines, -1 line	0 comments	Download
M	dart/frog/leg/tree/nodes.dart	View		3 chunks	+5 lines, -5 lines	0 comments	Download
M	dart/frog/leg/warnings.dart	View		2 chunks	+4 lines, -0 lines	0 comments	Download
M	dart/frog/tests/leg/src/ResolverTest.dart	View		6 chunks	+56 lines, -19 lines	0 comments	Download
M	dart/tests/co19/co19-leg.status	View		4 chunks	+19 lines, -22 lines	0 comments	Download
M	dart/tests/language/language-leg.status	View		2 chunks	+0 lines, -2 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

ngeoffray

LGTM! https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/elements/elements.dart File dart/frog/leg/elements/elements.dart (right): https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/elements/elements.dart#newcode13 dart/frog/leg/elements/elements.dart:13: static final int NONE = 0; I would ...

8 years, 9 months ago (2012-03-08 14:00:45 UTC) #2

karlklose

LGTM. https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/resolver.dart File dart/frog/leg/resolver.dart (right): https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/resolver.dart#newcode801 dart/frog/leg/resolver.dart:801: [receiverClass.name, name]); Would this fit into one line? ...

8 years, 9 months ago (2012-03-08 14:01:45 UTC) #3

karlklose

Could you rename SLOT to something like VARIABLE (and perhaps rename the Variable kind) in ...

8 years, 9 months ago (2012-03-08 15:04:51 UTC) #4

ahe

8 years, 9 months ago (2012-03-08 18:13:39 UTC) #5

Hi Karl and Nicolas,

Thank you for your ideas.

Cheers,
Peter

https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/elements/...
File dart/frog/leg/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/elements/...
dart/frog/leg/elements/elements.dart:13: static final int NONE = 0;
On 2012/03/08 14:00:45, ngeoffray wrote:
> I would add a comment on what has NONE category (getter, setter) and why.

Done.

https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/resolver....
File dart/frog/leg/resolver.dart (right):

https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/resolver....
dart/frog/leg/resolver.dart:784: if (currentClass === null ||
!inInstanceContext) {
On 2012/03/08 14:00:45, ngeoffray wrote:
> Shouldn't that just be '!inInstanceContext' ?

Done.

https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/resolver....
dart/frog/leg/resolver.dart:794: } else if (resolvedReceiver.kind ===
ElementKind.CLASS) {
On 2012/03/08 14:00:45, ngeoffray wrote:
> You can now change it to resolvedReceiver.isClass()

I'm not sure that I like that.

https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/resolver....
dart/frog/leg/resolver.dart:801: [receiverClass.name, name]);
On 2012/03/08 14:01:45, karlklose wrote:
> Would this fit into one line?

Done.

https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/resolver....
dart/frog/leg/resolver.dart:1237: }
On 2012/03/08 14:00:45, ngeoffray wrote:
> Why don't you warn here that you resolve to something not a prefix?

Done.

https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/tree/node...
File dart/frog/leg/tree/nodes.dart (right):

https://chromiumcodereview.appspot.com/9634008/diff/1/dart/frog/leg/tree/node...
dart/frog/leg/tree/nodes.dart:89: String unparse(bool printDebugInfo) {
On 2012/03/08 14:01:45, karlklose wrote:
> How about making this an optional argument?

I'm not sure I see the point?

Expand Messages | Collapse Messages