code review 13057043: go.tools/ssa: fixes, cleanups, cosmetic tweaks.

go.tools/ssa: fixes, cleanups, cosmetic tweaks.

Fix bug: the Signature for an interface method wrapper
erroneously had a non-nil receiver.

Function:
- Set Pkg field non-nil even for wrappers.
  It is equal to that of the wrapped function.
  Only wrappers of error.Error
  (and its embeddings in other interfaces) may have nil.
  Sanity checker now asserts this.
- FullName() now uses .Synthetic field to discriminate
  synthetic methods, not Pkg==nil.
- Fullname() uses new relType() utility to print receiver type
  name unqualified if it belongs to the same package.
  (Alloc.String also uses relType utility.)

CallCommon:
- Description(): fix switch logic broken when we
  eliminated the Recv field.
- better docs.

[adonovan, 2013-08-16 16:27:25]

Hello david.crawshaw@zentus.com (cc: golang-dev@googlegroups.com),

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

[crawshaw1, 2013-08-19 19:11:47]

LGTM

https://codereview.appspot.com/13057043/diff/1002/ssa/print.go
File ssa/print.go (right):

https://codereview.appspot.com/13057043/diff/1002/ssa/print.go#newcode40
ssa/print.go:40: // relType is like t.String(), but if t is a Named type (or
pointer^n
^n

https://codereview.appspot.com/13057043/diff/1002/ssa/print.go#newcode49
ssa/print.go:49: func relType(t types.Type, from *Package) string {
This is fine for the use you have, but if it is going to be used more widely as
your TODO suggests, I think this belongs in the types package.

Cases like "<-chan mypkg.T" will make this function significantly more complex.
There is also a TODO in go/types/errors.go:306 about this. Perhaps
t.StringForPkg(pkg string).

[adonovan, 2013-08-19 19:32:13]

On 2013/08/19 19:11:47, crawshaw1 wrote:
> LGTM
> 
> https://codereview.appspot.com/13057043/diff/1002/ssa/print.go
> File ssa/print.go (right):
> 
> https://codereview.appspot.com/13057043/diff/1002/ssa/print.go#newcode40
> ssa/print.go:40: // relType is like t.String(), but if t is a Named type (or
> pointer^n
> ^n

It was intentional, if cryptic.  Rephrased:

// relType is like t.String(), but if t is a Named type belonging to
// package from, optionally wrapped by one or more Pointer
// constructors, package qualification is suppressed.
 
> https://codereview.appspot.com/13057043/diff/1002/ssa/print.go#newcode49
> ssa/print.go:49: func relType(t types.Type, from *Package) string {
> This is fine for the use you have, but if it is going to be used more widely
as
> your TODO suggests, I think this belongs in the types package.
> 
> Cases like "<-chan mypkg.T" will make this function significantly more
complex.
> There is also a TODO in go/types/errors.go:306 about this. Perhaps
> t.StringForPkg(pkg string).

Couldn't agree more.  Added a TODO(gri).

[adonovan, 2013-08-19 19:38:32]

*** Submitted as
https://code.google.com/p/go/source/detail?r=b0c3f2b8db67&repo=tools ***

go.tools/ssa: fixes, cleanups, cosmetic tweaks.

Fix bug: the Signature for an interface method wrapper
erroneously had a non-nil receiver.

Function:
- Set Pkg field non-nil even for wrappers.
  It is equal to that of the wrapped function.
  Only wrappers of error.Error
  (and its embeddings in other interfaces) may have nil.
  Sanity checker now asserts this.
- FullName() now uses .Synthetic field to discriminate
  synthetic methods, not Pkg==nil.
- Fullname() uses new relType() utility to print receiver type
  name unqualified if it belongs to the same package.
  (Alloc.String also uses relType utility.)

CallCommon:
- Description(): fix switch logic broken when we
  eliminated the Recv field.
- better docs.

R=david.crawshaw, crawshaw, gri
CC=golang-dev
https://codereview.appspot.com/13057043