Issue 177633008: Better error message for assignment to final local variables

Issue 177633008: Better error message for assignment to final local variables (Closed)

Created:
6 years, 10 months ago by hausner

Modified:
6 years, 9 months ago

Reviewers:
Ivan Posva

CC:
reviews_dartlang.org, vm-dev_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Better error message for assignment to final local variables “No top-level getter found” becomes “cannot assign to final variable xxx”. Fixes issue 16782. R=iposva@google.com Committed: https://code.google.com/p/dart/source/detail?r=33028

Patch Set 1 #

Patch Set 2 : #

Total comments: 4

Created: 6 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+24 lines, -12 lines)			Patch
M	runtime/lib/errors_patch.dart	View	1	2 chunks	+11 lines, -3 lines	2 comments	Download
M	runtime/lib/invocation_mirror.h	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	runtime/lib/invocation_mirror_patch.dart	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	runtime/vm/parser.cc	View		1 chunk	+9 lines, -7 lines	2 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Ivan Posva

LGTM -Ivan https://codereview.chromium.org/177633008/diff/20001/runtime/lib/errors_patch.dart File runtime/lib/errors_patch.dart (right): https://codereview.chromium.org/177633008/diff/20001/runtime/lib/errors_patch.dart#newcode222 runtime/lib/errors_patch.dart:222: return "cannot assign to final variable '$memberName'.\n\n"; ...

6 years, 10 months ago (2014-02-25 07:10:07 UTC) #2

hausner

Committed patchset #2 manually as r33028 (presubmit successful).

6 years, 9 months ago (2014-02-25 18:01:42 UTC) #3

hausner

6 years, 9 months ago (2014-02-25 18:47:04 UTC) #4

Message was sent while issue was closed.

Missed the review comments. Formatting issue changed in

https://codereview.chromium.org/179083005

https://codereview.chromium.org/177633008/diff/20001/runtime/lib/errors_patch...
File runtime/lib/errors_patch.dart (right):

https://codereview.chromium.org/177633008/diff/20001/runtime/lib/errors_patch...
runtime/lib/errors_patch.dart:222: return "cannot assign to final variable
'$memberName'.\n\n";
On 2014/02/25 07:10:07, Ivan Posva wrote:
> How about just assigning to msg here and let the rest of the function deal
with
> massaging the resulting string?
I'm not sure what massaging you mean. Adding the two newlines?

The problem is that I'd have to special-case the local variable case again in
the switch below, or put the entire switch statement into an else branch. Is
that really more readable?

https://codereview.chromium.org/177633008/diff/20001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/177633008/diff/20001/runtime/vm/parser.cc#new...
runtime/vm/parser.cc:7876: original->token_pos(),
On 2014/02/25 07:10:07, Ivan Posva wrote:
> Bad indentation.

Moved parameters to the left. I find it unreadable to cram the conditional
expressions over 3 lines if the parameters are lined up with the opening paren
of the function call.

Expand Messages | Collapse Messages