code review 59140043: go.tools/go/types: print cyles of recursive type declar...

go.tools/go/types: print cyles of recursive type declarations

[gri, 2014-02-01 00:49:30]

Hello adonovan@google.com (cc: golang-codereviews@googlegroups.com),

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

[adonovan, 2014-02-03 17:58:09]

LGTM

Slightly surprised no tests were affected.  Were the changed error messages
matched by wildcards?

https://codereview.appspot.com/59140043/diff/40001/go/types/decl.go
File go/types/decl.go (left):

https://codereview.appspot.com/59140043/diff/40001/go/types/decl.go#oldcode124
go/types/decl.go:124: // TODO(gri) document arguments
A comment would still be good.

https://codereview.appspot.com/59140043/diff/40001/go/types/typexpr.go
File go/types/typexpr.go (right):

https://codereview.appspot.com/59140043/diff/40001/go/types/typexpr.go#newcode79
go/types/typexpr.go:79: check.errorf(obj.Pos(), "\t%s refers to", obj.Name()) //
secondary error, \t indented
It would be good to document this convention at types.Config.Error.

[gri, 2014-02-03 18:33:35]

The error tests don't change because such secondary error messages are not
checked.

I should add at least one or two tests that checks them as well. For another CL.

https://codereview.appspot.com/59140043/diff/40001/go/types/decl.go
File go/types/decl.go (left):

https://codereview.appspot.com/59140043/diff/40001/go/types/decl.go#oldcode124
go/types/decl.go:124: // TODO(gri) document arguments
On 2014/02/03 17:58:10, adonovan wrote:
> A comment would still be good.

will comment when I comment all the other functions

https://codereview.appspot.com/59140043/diff/40001/go/types/typexpr.go
File go/types/typexpr.go (right):

https://codereview.appspot.com/59140043/diff/40001/go/types/typexpr.go#newcode79
go/types/typexpr.go:79: check.errorf(obj.Pos(), "\t%s refers to", obj.Name()) //
secondary error, \t indented
On 2014/02/03 17:58:10, adonovan wrote:
> It would be good to document this convention at types.Config.Error.

Done.

[gri, 2014-02-03 18:34:10]

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

go.tools/go/types: print cyles of recursive type declarations

LGTM=adonovan
R=adonovan
CC=golang-codereviews
https://codereview.appspot.com/59140043