Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(4555)

Issue 180112: code review 180112: Allow %p on reference types, for debugging. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 2 months ago by r
Modified:
14 years, 2 months ago
Reviewers:
rsc
CC:
iant, golang-dev
Visibility:
Public.

Description

Allow %p on reference types, for debugging. (Also fix case sensitivity in test for PTR inside fmt_test.go) Fixes issue 441.

Patch Set 1 #

Patch Set 2 : code review 180112: Allow %p on reference types, for debugging. #

Patch Set 3 : code review 180112: Allow %p on reference types, for debugging. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -18 lines) Patch
M src/pkg/fmt/fmt_test.go View 1 2 chunks +6 lines, -1 line 0 comments Download
M src/pkg/fmt/print.go View 2 chunks +9 lines, -17 lines 1 comment Download

Messages

Total messages: 4
r
Hello (cc: golang-dev@googlegroups.com), I'd like you to review the following change.
14 years, 2 months ago (2009-12-22 05:18:10 UTC) #1
iant
LGTM
14 years, 2 months ago (2009-12-22 05:43:44 UTC) #2
r
*** Submitted as http://code.google.com/p/go/source/detail?r=e526f4d7d93c *** Allow %p on reference types, for debugging. (Also fix case ...
14 years, 2 months ago (2009-12-22 06:02:04 UTC) #3
rsc
14 years, 2 months ago (2009-12-22 16:49:16 UTC) #4
http://codereview.appspot.com/180112/diff/8/1007
File src/pkg/fmt/print.go (right):

http://codereview.appspot.com/180112/diff/8/1007#newcode811
src/pkg/fmt/print.go:811: p.fmt.fmt_uX64(uint64(field.Addr()))
Addr() is a pointer to the chan, map, or slice value,
not the chan, map, or slice value itself.

I think here you want

switch v := field.(type) {
case interface { Get() uintptr }:
    ...
default:
    goto badtype
}

That will handle PtrValue and ChanValue now,
and then MapValue if you add MapValue.Get
(can just copy ChanValue.Get).
You could add SliceValue.Get too, though 
perhaps the %p format of a SliceValue should
show len and cap too.

If %p formats SliceValue it should probably also
format StringValue.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b