code review 4332041: test: adjust bug324 to expect run-time failure, not com...

test: adjust bug324 to expect run-time failure, not compile-time.

Failing at compile time requires that for each conversion
between two interface types the compiler compare the sets of
unexported methods to see if they come from different
packages.  Since this test will fail approximately never on
real code, and since it can't catch all cases of the problem,
I don't think it's worth testing in the compiler.  This CL
changes this test to look for a run-time panic rather than a
compile-time error.

[iant, 2011-03-29 13:55:28]

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

I'd like you to review this change to
https://go.googlecode.com/hg/

[rsc1, 2011-03-29 14:07:47]

I am not so sure.

Granted, I haven't fixed this bug yet in 6g,
but it seems to me not that different from
any other interface check.  If the compiler
doesn't do this check, then that means it
doesn't reject, say, passing an ast.Stmt
where an ast.Expr is expected: those interfaces
differ only in their unexported methods.

Russ

[rsc1, 2011-03-29 14:14:09]

Also, what do you mean by "can't catch all instances
of the problem"?

Russ

[iant2, 2011-03-29 15:04:15]

Russ Cox <rsc@google.com> writes:

> Granted, I haven't fixed this bug yet in 6g,
> but it seems to me not that different from
> any other interface check.  If the compiler
> doesn't do this check, then that means it
> doesn't reject, say, passing an ast.Stmt
> where an ast.Expr is expected: those interfaces
> differ only in their unexported methods.

I think this is a different case.  The only way to detect this error at
compile time is to say "I know this object implements this interface
defined in package p1; since that includes an unexported name, I can
conclude that this object can not implement the interface defined in
package p2, which also includes an unexported name."  ast.Stmt and
ast.Expr are in the same package; there is merely a matter of checking
which methods they define.

> Also, what do you mean by "can't catch all instances
> of the problem"?

You still have to check at run-time.  That is, we can't eliminate the
run-time check by a compile time check.

Ian

[rsc1, 2011-03-29 18:22:15]

On Tue, Mar 29, 2011 at 11:03, Ian Lance Taylor <iant@google.com> wrote:
> I think this is a different case.  The only way to detect this error at
> compile time is to say "I know this object implements this interface
> defined in package p1; since that includes an unexported name, I can
> conclude that this object can not implement the interface defined in
> package p2, which also includes an unexported name."

I agree now.  I missed that px was an interface.
I was thinking it was a *p.Implementation, even
though the type assertion obviously requires an
interface.

While you are there could you please uncomment
the "correctly illegal" assignments and give them
appropriate ERROR lines.

Russ

[iant2, 2011-03-29 18:50:45]

Russ Cox <rsc@google.com> writes:

> On Tue, Mar 29, 2011 at 11:03, Ian Lance Taylor <iant@google.com> wrote:
>> I think this is a different case.  The only way to detect this error at
>> compile time is to say "I know this object implements this interface
>> defined in package p1; since that includes an unexported name, I can
>> conclude that this object can not implement the interface defined in
>> package p2, which also includes an unexported name."
>
> I agree now.  I missed that px was an interface.
> I was thinking it was a *p.Implementation, even
> though the type assertion obviously requires an
> interface.
>
> While you are there could you please uncomment
> the "correctly illegal" assignments and give them
> appropriate ERROR lines.

In this CL I've converted this bug to test the runtime behaviour, so it
shouldn't have ERROR lines.

I just created http://codereview.appspot.com/4271086 to check that the
error cases are caught at compile time.

Ian

[rsc, 2011-03-29 20:44:46]

LGTM

[iant, 2011-03-29 22:03:14]

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

test: adjust bug324 to expect run-time failure, not compile-time.

Failing at compile time requires that for each conversion
between two interface types the compiler compare the sets of
unexported methods to see if they come from different
packages.  Since this test will fail approximately never on
real code, and since it can't catch all cases of the problem,
I don't think it's worth testing in the compiler.  This CL
changes this test to look for a run-time panic rather than a
compile-time error.

R=gri, rsc1, iant2, rsc
CC=golang-dev
http://codereview.appspot.com/4332041