code review 6500064: cmd/gc: Suggest *T in error for x.(T) if it would work.

cmd/gc: Suggest *T in error for x.(T) if it would work.

Accomplished by synchronizing the formatting of conversion errors between typecheck.c and subr.c

Fixes issue 3984.

[DMorsing, 2012-09-01 13:06:22]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
rsc@golang.org),

I'd like you to review this change to
https://code.google.com/p/go/

[remyoudompheng, 2012-09-01 13:20:40]

http://codereview.appspot.com/6500064/diff/4001/src/cmd/gc/typecheck.c
File src/cmd/gc/typecheck.c (right):

http://codereview.appspot.com/6500064/diff/4001/src/cmd/gc/typecheck.c#newcod...
src/cmd/gc/typecheck.c:744: yyerror("impossible type assertion: %lN cannot have
dynamic type %T"
is it possible to uniformize the message with subr.c:1204, which says "%S method
requires pointer receiver"?

I'm not sure it is easier to understand but I'm not sure either why the messages
should be different.

[rsc, 2012-09-01 14:15:38]

http://codereview.appspot.com/6500064/diff/4001/src/cmd/gc/typecheck.c
File src/cmd/gc/typecheck.c (right):

http://codereview.appspot.com/6500064/diff/4001/src/cmd/gc/typecheck.c#newcod...
src/cmd/gc/typecheck.c:744: yyerror("impossible type assertion: %lN cannot have
dynamic type %T"
I like this suggestion. Probably this whole if else chain should be revised to
match the one in subr.c:1204, with the same sequence of conditions and the same
descriptive suffixes.

[DMorsing, 2012-09-01 15:02:14]

http://codereview.appspot.com/6500064/diff/4001/src/cmd/gc/typecheck.c
File src/cmd/gc/typecheck.c (right):

http://codereview.appspot.com/6500064/diff/4001/src/cmd/gc/typecheck.c#newcod...
src/cmd/gc/typecheck.c:744: yyerror("impossible type assertion: %lN cannot have
dynamic type %T"
On 2012/09/01 14:15:38, rsc wrote:
> I like this suggestion. Probably this whole if else chain should be revised to
> match the one in subr.c:1204, with the same sequence of conditions and the
same
> descriptive suffixes.

If we do that, you could just move the formatting of the descriptive suffix into
implements(). It would get rid of duplicated code and make it simpler.

Still, I'm not sure if the suffixes can be written in such a way that they're
unambiguous for all the various contexts that conversion exists in.

[rsc, 2012-09-01 15:08:40]

> Still, I'm not sure if the suffixes can be written in such a way that
> they're unambiguous for all the various contexts that conversion exists
> in.

I'd start by copying the logic and suffixes in this one case and leave
refactoring for later.

Russ

[DMorsing, 2012-09-01 15:45:53]

Copied over the implementation from subr.c to typecheck.c and updated the CL
description to match.

PTAL.

[rsc, 2012-09-01 16:59:11]

LGTM

Thanks!

[rsc, 2012-09-01 17:53:00]

*** Submitted as http://code.google.com/p/go/source/detail?r=33a62d86ca2f ***

cmd/gc: Suggest *T in error for x.(T) if it would work.

Accomplished by synchronizing the formatting of conversion errors between
typecheck.c and subr.c

Fixes issue 3984.

R=golang-dev, remyoudompheng, rsc
CC=golang-dev
http://codereview.appspot.com/6500064

Committer: Russ Cox <rsc@golang.org>