code review 3435042: Fixes issue 1324.

Fixes issue 1324.

[lstoakes, 2010-12-09 00:18:27]

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

I'd like you to review this change.

[rsc1, 2010-12-09 18:27:48]

Issue 1324 is not a bug.

[rsc1, 2010-12-09 18:33:21]

Actually, I take it back.  There is a bug.
But I am not yet sure what the bug is:
the error or the error message.
This CL only changes the error message.

Russ

[lstoakes, 2010-12-09 18:55:10]

On Thursday, 9 December 2010, Russ Cox <rsc@google.com> wrote:
> Actually, I take it back.  There is a bug.
> But I am not yet sure what the bug is:
> the error or the error message.
> This CL only changes the error message.
>

Perhaps listing both the pointer type and the original type in the
error message is the way to go? I can modify and re-submit accordingly
if required.

-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[rsc1, 2010-12-09 19:52:02]

See issue; we believe the error is incorrect
(the program is valid).

Russ

[lstoakes, 2011-01-01 12:34:02]

On 9 December 2010 19:51, Russ Cox <rsc@google.com> wrote:
> See issue; we believe the error is incorrect
> (the program is valid).

Okay, I have a patch which appears to fix this, however bug117.go in
test/fixedbugs specifically checks whether a pointer type's methods
are resolved from its underlying type, and fails if so, which is
precisely what the patch is making the case :)

So is it definitely the case that we want to be able to access the
methods of the underlying type of a pointer type, in which case this
is not a bug and should be removed, or is this test failure valid, in
which case what is the way forward with resolving this bug?

-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[lstoakes, 2011-01-05 00:49:13]

Would it be easier for me to just submit the patch? :)

On 1 January 2011 12:33, Lorenzo Stoakes <lstoakes@gmail.com> wrote:
> On 9 December 2010 19:51, Russ Cox <rsc@google.com> wrote:
>> See issue; we believe the error is incorrect
>> (the program is valid).
>
> Okay, I have a patch which appears to fix this, however bug117.go in
> test/fixedbugs specifically checks whether a pointer type's methods
> are resolved from its underlying type, and fails if so, which is
> precisely what the patch is making the case :)
>
> So is it definitely the case that we want to be able to access the
> methods of the underlying type of a pointer type, in which case this
> is not a bug and should be removed, or is this test failure valid, in
> which case what is the way forward with resolving this bug?
>
> --
> Lorenzo Stoakes
> http://www.codegrunt.co.uk
>



-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[rsc1, 2011-01-05 01:56:05]

Sure.  I haven't had a chance to dig into the test case,
though I did get so far as opening the file today.  :-)

[lstoakes, 2011-01-05 08:05:21]

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

Please take another look.

[rsc1, 2011-01-05 17:33:12]

There should be a test.

Russ

[lstoakes, 2011-01-05 18:18:08]

I will add one. Actually I notice now that bug117.go is generating an
internal compiler error (whereas it should now compile fine) - I will
look at fixing this and submit an updated patch along with a test when
done.

Lorenzo

On 5 January 2011 17:33, Russ Cox <rsc@google.com> wrote:
> There should be a test.
>
> Russ
>



-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[lstoakes, 2011-01-06 00:04:50]

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

Please take another look.

[lstoakes, 2011-01-06 00:06:54]

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

Please take another look.

[lstoakes, 2011-01-06 00:08:16]

Apologies for the duplication, having issues getting mercurial to
recognise I have added a test file as well as modified bug117.go.

Lorenzo

On 6 January 2011 00:06,  <lstoakes@gmail.com> wrote:
> Hello rsc1 (cc: golang-dev@googlegroups.com),
>
> Please take another look.
>
>
> http://codereview.appspot.com/3435042/
>



-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[lstoakes, 2011-01-06 00:10:12]

Ok; it appears I can send them as a separate patch; will do that.

On 6 January 2011 00:07, Lorenzo Stoakes <lstoakes@gmail.com> wrote:
> Apologies for the duplication, having issues getting mercurial to
> recognise I have added a test file as well as modified bug117.go.
>
> Lorenzo
>
> On 6 January 2011 00:06,  <lstoakes@gmail.com> wrote:
>> Hello rsc1 (cc: golang-dev@googlegroups.com),
>>
>> Please take another look.
>>
>>
>> http://codereview.appspot.com/3435042/
>>
>
>
>
> --
> Lorenzo Stoakes
> http://www.codegrunt.co.uk
>



-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[lstoakes, 2011-01-06 00:19:51]

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

Please take another look.

[lstoakes, 2011-01-06 00:20:58]

Cool, all done (as you can see below :-)

Still getting used to all this, so forgive my being slow on the uptake!!

Lorenzo

On 6 January 2011 00:19,  <lstoakes@gmail.com> wrote:
> Hello rsc1 (cc: golang-dev@googlegroups.com),
>
> Please take another look.
>
>
> http://codereview.appspot.com/3435042/
>



-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[lstoakes, 2011-01-06 14:39:19]

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

Please take another look.

[lstoakes, 2011-01-06 14:42:11]

Cleaned up typecheck.c patch - had inadvertently added unwanted space
and changed indentation; I need to transform spaces -> tabs, will
perform one more update to fix that.

Lorenzo

On 6 January 2011 14:39,  <lstoakes@gmail.com> wrote:
> Hello rsc1 (cc: golang-dev@googlegroups.com),
>
> Please take another look.
>
>
> http://codereview.appspot.com/3435042/
>



-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[lstoakes, 2011-01-06 14:44:08]

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

Please take another look.

[rsc1, 2011-01-06 15:45:29]

Thanks for identifying that bug117 changes.
We need to decide what the language spec is before
we can apply this change.  I've updated the issue.

Russ

[lstoakes, 2011-01-12 09:50:11]

Any movement on this at all? I am happy to modify the error message to
be more readable if bug117.go ought to legitimately cause an error :-)

Cheers, Lorenzo

On 6 January 2011 15:45, Russ Cox <rsc@google.com> wrote:
> Thanks for identifying that bug117 changes.
> We need to decide what the language spec is before
> we can apply this change.  I've updated the issue.
>
> Russ
>



-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[rsc1, 2011-01-13 18:17:35]

No, there hasn't been any movement.
I think this is going to be rather low priority.

Russ

[lstoakes, 2011-01-13 20:20:48]

Okay, no probs :)

Lorenzo

On 13 January 2011 18:17, Russ Cox <rsc@google.com> wrote:
> No, there hasn't been any movement.
> I think this is going to be rather low priority.
>
> Russ
>



-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[lstoakes, 2011-02-08 06:09:00]

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

Please take another look.

[lstoakes, 2011-02-08 06:10:43]

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

Please take another look.

[lstoakes, 2011-02-10 11:12:41]

Just to clarify; given the discussion over at the issue page for issue
1324 (http://code.google.com/p/go/issues/detail?id=1324), this CL
implements a change in error message to report the correct type in the
following scenario:-

type T struct{}
type P *T

func (t *T) Meth() {}
func (t T) Meth2() {}

func main() {
	t := &T{}
	p := P(t)
	p.Meth()  // error
	p.Meth2() // error
}

Currently, go reports:-

p.Meth undefined (type T has no field or method Meth)
p.Meth2 undefined (type T has no field or method Meth2)

However this error message is incorrect, as the problem is not that T
has no field/method Meth/Meth2, but rather that P doesn't. The CL
fixes the error message to say:-

p.Meth undefined (type P has no field or method Meth)
p.Meth2 undefined (type P has no field or method Meth2)

- Lorenzo

On 8 February 2011 06:10,  <lstoakes@gmail.com> wrote:
> Hello rsc1 (cc: golang-dev@googlegroups.com),
>
> Please take another look.
>
>
> http://codereview.appspot.com/3435042/
>



-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[rsc1, 2011-02-11 21:47:04]

LGTM

Thanks for seeing this through.

[lstoakes, 2011-02-11 22:20:58]

On 11 February 2011 21:47, Russ Cox <rsc@google.com> wrote:
> LGTM
>
> Thanks for seeing this through.
>

No problem, glad to be able to contribute to an awesome project :)
hope to contribute some more in the future too.

-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[r, 2011-02-11 22:44:19]

Please add explanatory info to the CL description so it's useful in isolation.

[rsc, 2011-02-11 22:48:00]

*** Submitted as 2562ce5a7a4b ***

gc: correct receiver in method missing error

Fixes issue 1324.

R=rsc1, r, rsc
CC=golang-dev
http://codereview.appspot.com/3435042

Committer: Russ Cox <rsc@golang.org>

[lstoakes, 2011-02-11 23:11:45]

On 11 February 2011 22:44, Rob 'Commander' Pike <r@golang.org> wrote:
> Please add explanatory info to the CL description so it's useful in isolation.
>

The issue has been closed; is there a way of doing this other than via
an hg update? Sorry I am rather new to the process :)

-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[r2, 2011-02-12 00:11:38]

On Feb 11, 2011, at 3:11 PM, Lorenzo Stoakes wrote:

> On 11 February 2011 22:44, Rob 'Commander' Pike <r@golang.org> wrote:
>> Please add explanatory info to the CL description so it's useful in
isolation.
>> 
> 
> The issue has been closed; is there a way of doing this other than via
> an hg update? Sorry I am rather new to the process :)
> 
> -- 
> Lorenzo Stoakes
> http://www.codegrunt.co.uk

It's easy to do before you submit; you can do it right on the codereview site,
or just run hg change and edit the text. After it's submitted, well, it's
submitted.


-rob

[rsc1, 2011-02-12 00:17:25]

I did it before I submitted.

Russ
On Feb 11, 2011 7:11 PM, "Rob &apos;Commander&apos; Pike" <r@google.com>
wrote:
>
> On Feb 11, 2011, at 3:11 PM, Lorenzo Stoakes wrote:
>
>> On 11 February 2011 22:44, Rob 'Commander' Pike <r@golang.org> wrote:
>>> Please add explanatory info to the CL description so it's useful in
isolation.
>>>
>>
>> The issue has been closed; is there a way of doing this other than via
>> an hg update? Sorry I am rather new to the process :)
>>
>> --
>> Lorenzo Stoakes
>> http://www.codegrunt.co.uk
>
> It's easy to do before you submit; you can do it right on the codereview
site, or just run hg change and edit the text. After it's submitted, well,
it's submitted.
>
>
> -rob
>

[r2, 2011-02-12 00:19:42]

http://codereview.appspot.com/3435042/ doesn't show any edit

-rob

[rsc1, 2011-02-12 02:17:35]

On Fri, Feb 11, 2011 at 19:19, Rob 'Commander' Pike <r@google.com> wrote:
> http://codereview.appspot.com/3435042/ doesn't show any edit

no, i can't change that.  but i ran hg change
between clpatch and submit so that the log
entry has the new description:
http://code.google.com/p/go/source/detail?r=2562ce5a7a4bb7

it was in the submitted mail too:

*** Submitted as 2562ce5a7a4b ***

gc: correct receiver in method missing error

Fixes issue 1324.

R=rsc1, r, rsc
CC=golang-dev

http://codereview.appspot.com/3435042

Committer: Russ Cox <rsc@golang.org>