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

Issue 9169008: Do not resolve the selector to find the getter, but the receiver instead. (Closed)

Created:
8 years, 11 months ago by ngeoffray
Modified:
8 years, 11 months ago
Reviewers:
karlklose, floitsch, ahe
CC:
reviews_dartlang.org, kasperl, ahe, Lasse Reichstein Nielsen
Visibility:
Public.

Description

Looking at the selector was wrong initially. Keep the TODO but just use the target (which is the fied currently because we don't have setters). Committed: https://code.google.com/p/dart/source/detail?r=3132

Patch Set 1 : '' #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -4 lines) Patch
M frog/leg/resolver.dart View 1 chunk +1 line, -1 line 2 comments Download
M frog/leg/ssa/builder.dart View 1 chunk +1 line, -1 line 0 comments Download
A frog/tests/leg_only/src/GetterElementTest.dart View 1 chunk +29 lines, -0 lines 0 comments Download
M tests/language/language-leg.status View 2 chunks +0 lines, -2 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
ngeoffray
8 years, 11 months ago (2012-01-10 09:45:57 UTC) #1
karlklose
LGTM!
8 years, 11 months ago (2012-01-10 09:54:40 UTC) #2
ahe
DBC http://codereview.chromium.org/9169008/diff/2001/frog/leg/resolver.dart File frog/leg/resolver.dart (right): http://codereview.chromium.org/9169008/diff/2001/frog/leg/resolver.dart#newcode373 frog/leg/resolver.dart:373: if (node.isIndex) { I don't think there is ...
8 years, 11 months ago (2012-01-10 11:42:25 UTC) #3
ngeoffray
8 years, 11 months ago (2012-01-10 11:45:35 UTC) #4
http://codereview.chromium.org/9169008/diff/2001/frog/leg/resolver.dart
File frog/leg/resolver.dart (right):

http://codereview.chromium.org/9169008/diff/2001/frog/leg/resolver.dart#newco...
frog/leg/resolver.dart:373: if (node.isIndex) {
On 2012/01/10 11:42:26, ahe wrote:
> I don't think there is a need for this if-test.

Indeed, I just wanted to have different code paths for both, letting me have the
TODO there, and refactor once we know how to handle real setters.

Powered by Google App Engine
This is Rietveld 408576698