Issue 9695059: Fix leg assert and NPE to make BB green.

Issue 9695059: Fix leg assert and NPE to make BB green. (Closed)

Created:
8 years, 9 months ago by ngeoffray

Modified:
8 years, 9 months ago

Reviewers:
ahe, floitsch

CC:
reviews_dartlang.org, karlklose, Lasse Reichstein Nielsen, kasperl

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

Visibility:
Public.

More Reviews

Description

Fix leg assert and NPE to make BB green. Committed: https://code.google.com/p/dart/source/detail?r=5434

Patch Set 1 #

Total comments: 3

Created: 8 years, 9 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+8 lines, -5 lines)			Patch
M	frog/leg/elements/elements.dart	View	1 chunk	+2 lines, -1 line	3 comments	Download
M	frog/leg/ssa/closure.dart	View	1 chunk	+4 lines, -2 lines	0 comments	Download
M	samples/tests/samples/samples-leg.status	View	1 chunk	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

ngeoffray

TBR https://chromiumcodereview.appspot.com/9695059/diff/1/frog/leg/elements/elements.dart File frog/leg/elements/elements.dart (right): https://chromiumcodereview.appspot.com/9695059/diff/1/frog/leg/elements/elements.dart#newcode429 frog/leg/elements/elements.dart:429: // TODO(ahe): checking for null should not be ...

8 years, 9 months ago (2012-03-13 20:08:24 UTC) #1

floitsch

LGTM. https://chromiumcodereview.appspot.com/9695059/diff/1/frog/leg/elements/elements.dart File frog/leg/elements/elements.dart (right): https://chromiumcodereview.appspot.com/9695059/diff/1/frog/leg/elements/elements.dart#newcode431 frog/leg/elements/elements.dart:431: } else { return null; }

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

ahe

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

Sorry, this does not look good :-(

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

https://chromiumcodereview.appspot.com/9695059/diff/1/frog/leg/elements/eleme...
frog/leg/elements/elements.dart:429: // TODO(ahe): checking for null should not
be necessary.
On 2012/03/13 20:08:24, ngeoffray wrote:
> Told you so :-)

Why is this happening? I think this is a symptom fix, that masks a different
bug.

The method [position] may not return null. That will lead to a crash in a
different place.

Take a look at this code in ContainerElement.addGetterOrSetter:

      AbstractFieldElement field = new AbstractFieldElement(element.name, this);
      addMember(field, listener);
      if (element.kind == ElementKind.GETTER) {
        field.getter = element;
      } else {
        field.setter = element;
      }

I think the problem is that [addMember] is called before the
AbstractFieldElement is set up correctly, and you will find that the error
message is not indicating the correct location.

Expand Messages | Collapse Messages