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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 9193016: Add the arity to calls, and support noSuchMethodException. (Closed)

Created:
8 years, 11 months ago by ngeoffray
Modified:
8 years, 11 months ago
CC:
reviews_dartlang.org, karlklose, ahe
Visibility:
Public.

Description

Add the arity to calls, and support noSuchMethodException. Committed: https://code.google.com/p/dart/source/detail?r=3540

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 18

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+283 lines, -134 lines) Patch
M frog/leg/compiler.dart View 1 2 4 chunks +19 lines, -7 lines 0 comments Download
M frog/leg/emitter.dart View 1 2 4 chunks +53 lines, -4 lines 0 comments Download
M frog/leg/lib/core.dart View 1 2 2 chunks +32 lines, -0 lines 0 comments Download
M frog/leg/namer.dart View 1 2 4 chunks +72 lines, -80 lines 0 comments Download
M frog/leg/ssa/builder.dart View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M frog/leg/ssa/codegen.dart View 1 2 3 chunks +13 lines, -4 lines 0 comments Download
M frog/leg/universe.dart View 1 2 1 chunk +6 lines, -6 lines 0 comments Download
M frog/tests/leg/src/ArrayStaticInterceptTest.dart View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M frog/tests/leg/src/ClassCodegenTest.dart View 1 2 2 chunks +11 lines, -9 lines 0 comments Download
M frog/tests/leg/src/ClosureCodegenTest.dart View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M frog/tests/leg/src/ConstantFoldingTest.dart View 1 2 1 chunk +4 lines, -4 lines 0 comments Download
M frog/tests/leg/src/LiteralListTest.dart View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M frog/tests/leg/src/PrettyParameterTest.dart View 1 2 1 chunk +4 lines, -4 lines 0 comments Download
M frog/tests/leg/src/SsaPhiEliminatorTest.dart View 1 2 1 chunk +1 line, -1 line 0 comments Download
M frog/tests/leg/src/TypeGuardUnuserTest.dart View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M frog/tests/leg/src/mock_compiler.dart View 1 2 1 chunk +1 line, -1 line 0 comments Download
A frog/tests/leg_only/src/NoSuchMethodTest.dart View 1 1 chunk +51 lines, -0 lines 0 comments Download
M tests/corelib/corelib-leg.status View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
M tests/language/language-leg.status View 1 2 2 chunks +1 line, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
ngeoffray
8 years, 11 months ago (2012-01-23 17:43:26 UTC) #1
floitsch
LGTM! https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/emitter.dart File frog/leg/emitter.dart (right): https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/emitter.dart#newcode188 frog/leg/emitter.dart:188: if (!addedInheritFunction) return; I assume you don't want ...
8 years, 11 months ago (2012-01-24 11:46:03 UTC) #2
ngeoffray
8 years, 11 months ago (2012-01-24 12:09:38 UTC) #3 
Thanks Florian!

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/emitter.dart
File frog/leg/emitter.dart (right):

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/emitter.dar...
frog/leg/emitter.dart:188: if (!addedInheritFunction) return;
On 2012/01/24 11:46:03, floitsch wrote:
> Add TODO that we don't need to generate these methods if nobody overwrites
> noSuchMethod ?

Done.

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/emitter.dar...
frog/leg/emitter.dart:188: if (!addedInheritFunction) return;
On 2012/01/24 11:46:03, floitsch wrote:
> I assume you don't want to use instantiatedClasses because it contains native
> classes?

No, I just didn't think of using it (addedInheritFunction was local to this
class, so simpler to use).

> Otherwise the following would be cleaner:
> if (!compiler.universe.instantiatedClasses.isEmpty()) ...

I agree. Changed to use instantiatedClasses.

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/lib/core.dart
File frog/leg/lib/core.dart (right):

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/lib/core.da...
frog/leg/lib/core.dart:344: : receiver = r, name = n, args = a;
On 2012/01/24 11:46:03, floitsch wrote:
> I don't really care but I thought that : is indented by 4.

It doesn't look consistent across our code base, but the corelib seems to use 4,
so using 4.

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/namer.dart
File frog/leg/namer.dart (right):

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/namer.dart#...
frog/leg/namer.dart:34: return '\$call\$$arity';
On 2012/01/24 11:46:03, floitsch wrote:
> return instanceMethodeName(const SourceString(@'$call'), arity);

Done.

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/ssa/builder...
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:533: new FunctionElement(const SourceString('\$call'),
On 2012/01/24 11:46:03, floitsch wrote:
> I would prefer keeping the '\$call' local to some place.
> I chose the namer for it, but if you prefer another place I'm fine with it.
> Maybe a static namer.CLOSURE_INVOCATION_NAME = const SourceString('\$call'); ?

Done.

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/ssa/codegen...
File frog/leg/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/leg/ssa/codegen...
frog/leg/ssa/codegen.dart:416: if (node.inputs[0] is !HForeignNew) {
On 2012/01/24 11:46:03, floitsch wrote:
> Add comment that this avoids adding the generative constructor bodies to the
> list of seen selectors.

Done.

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/tests/leg/src/C...
File frog/tests/leg/src/ConstantFoldingTest.dart (right):

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/tests/leg/src/C...
frog/tests/leg/src/ConstantFoldingTest.dart:41: NUMBER_FOLDING, 'main', const
RegExp("print\\\$1\\(7\\)"));
On 2012/01/24 11:46:03, floitsch wrote:
> please switch to @ strings.

Done.

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/tests/leg/src/C...
frog/tests/leg/src/ConstantFoldingTest.dart:43: NEGATIVE_NUMBER_FOLDING, 'main',
const RegExp("print\\\$1\\(1\\)"));
On 2012/01/24 11:46:03, floitsch wrote:
> ditto

Done.

https://chromiumcodereview.appspot.com/9193016/diff/2001/frog/tests/leg/src/C...
frog/tests/leg/src/ConstantFoldingTest.dart:46: RegExp regexp = const
RegExp('eqNull\\\$1\\(a\\)');
On 2012/01/24 11:46:03, floitsch wrote:
> ditto.

Done.

Powered by Google App Engine
This is Rietveld 408576698