Issue 23533040: Check for non-final field in the face of const constructors.

Issue 23533040: Check for non-final field in the face of const constructors. (Closed)

Created:
7 years, 3 months ago by Johnni Winther

Modified:
7 years, 3 months ago

Reviewers:
karlklose

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Check for non-final field in the face of const constructors. BUG=http://dartbug.com/12900,http://dartbug.com/12901 R=karlklose@google.com Committed: https://code.google.com/p/dart/source/detail?r=27227

Patch Set 3 : Updated cf. comments. #

Created: 7 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+128 lines, -29 lines)			Patch
M	sdk/lib/_internal/compiler/implementation/resolution/members.dart	View	1 2	3 chunks	+28 lines, -1 line	0 comments	Download
M	sdk/lib/_internal/compiler/implementation/warnings.dart	View	1 2	1 chunk	+23 lines, -0 lines	0 comments	Download
M	sdk/lib/_internal/lib/foreign_helper.dart	View		1 chunk	+1 line, -1 line	0 comments	Download
M	tests/co19/co19-dart2dart.status	View	1	1 chunk	+0 lines, -1 line	0 comments	Download
M	tests/co19/co19-dart2js.status	View	1	1 chunk	+0 lines, -1 line	0 comments	Download
M	tests/compiler/dart2js/message_kind_helper.dart	View		1 chunk	+8 lines, -3 lines	0 comments	Download
M	tests/compiler/dart2js/mock_compiler.dart	View		1 chunk	+16 lines, -0 lines	0 comments	Download
M	tests/compiler/dart2js/resolver_test.dart	View		3 chunks	+48 lines, -0 lines	0 comments	Download
M	tests/compiler/dart2js/type_checker_test.dart	View		3 chunks	+4 lines, -20 lines	0 comments	Download
M	tests/language/language_dart2js.status	View	1	2 chunks	+0 lines, -2 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

karlklose

LGTM. https://codereview.chromium.org/23533040/diff/1/sdk/lib/_internal/compiler/implementation/resolution/members.dart File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right): https://codereview.chromium.org/23533040/diff/1/sdk/lib/_internal/compiler/implementation/resolution/members.dart#newcode339 sdk/lib/_internal/compiler/implementation/resolution/members.dart:339: assert(isConstructor || element.isFactoryConstructor()); Remember to remove this when ...

7 years, 3 months ago (2013-09-05 12:43:42 UTC) #2

Johnni Winther

Committed patchset #3 manually as r27227 (presubmit successful).

7 years, 3 months ago (2013-09-06 06:21:05 UTC) #3

Johnni Winther

7 years, 3 months ago (2013-09-06 06:22:40 UTC) #4

Message was sent while issue was closed.

https://codereview.chromium.org/23533040/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/23533040/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:339:
assert(isConstructor || element.isFactoryConstructor());
On 2013/09/05 12:43:42, karlklose wrote:
> Remember to remove this when rebasing with my change.

Done.

https://codereview.chromium.org/23533040/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:710:
compiler.reportError(cls,
On 2013/09/05 12:43:42, karlklose wrote:
> Move the call out of the if and compute the Spannable as:
>   Spannable span = constConstructors.length > 1
>       ? cls : constConstructors[0];

Done.

https://codereview.chromium.org/23533040/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/warnings.dart (right):

https://codereview.chromium.org/23533040/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/warnings.dart:290: const
MessageKind('Info: This non-final field is blocking const '
On 2013/09/05 12:43:42, karlklose wrote:
> 'prevents using const constructors'?

Done.

https://codereview.chromium.org/23533040/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/warnings.dart:293: static const
MessageKind CONST_CONSTRUCTOR_WITH_NONFINAL_FIELDS_CONSTRUCTOR =
On 2013/09/05 12:43:42, karlklose wrote:
> And something similar here.

Done.

Expand Messages | Collapse Messages