Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fmt: converts numeric type to string before rendering characters as hex #34351

Closed
wti opened this issue Sep 17, 2019 · 6 comments
Closed

fmt: converts numeric type to string before rendering characters as hex #34351

wti opened this issue Sep 17, 2019 · 6 comments

Comments

@wti
Copy link

@wti wti commented Sep 17, 2019

What version of Go are you using (go version)?

$ go version
go version go1.13 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

same result with multiple systems

What did you do?

https://play.golang.org/p/MST7hKUpWmd

What did you expect to see?

number printed as hex

What did you see instead?

Stringer rendering of number printed as hex

Discussion

I defined a uint64 type (type shex uint64) and made it a Stringer.

When I print it using %x, the value is converted to string first, and then the characters are converted to hex. I expect it to be treated as a number and converted to hex.

A workaround is to cast it back to uint64, but that works only if you write the formatting code.

The code is at print.go:627:

// If a string is acceptable according to the format, see if
// the value satisfies one of the string-valued interfaces.
// Println etc. set verb to %v, which is "stringable".
switch verb {
     case 'v', 's', 'x', 'X', 'q':
    // Is it an error or Stringer?
    ...
   switch v := p.arg.(type) {
        ...
        case Stringer:
            handled = true
            defer p.catchPanic(p.arg, verb, "String")
            p.fmtString(v.String(), verb)
            return

Using Stringer seems reasonable for verbs s, q, and v, but incorrect for x/X. (Indeed, if x is there, why is d not?)

Motivation

I'm using a type defined on uint64 as an encoding of other values, and the String() implementation must render the decoded form. I will not control the code that renders these values, so I cannot convert the values back to uint64 before the hex form is printed. Code using my types should be able to print the numeric form using numeric verbs and the rendered/decoded form using string verbs.

@toothrot
Copy link
Contributor

@toothrot toothrot commented Sep 17, 2019

Hey @wti,

Thanks for filing this issue. As far as I can tell, this is consistent with the documentation for fmt: https://golang.org/pkg/fmt/.

String and slice of bytes (treated equivalently with these verbs):

%s the uninterpreted bytes of the string or slice
%q a double-quoted string safely escaped with Go syntax
%x base 16, lower-case, two characters per byte
%X base 16, upper-case, two characters per byte

Except when printed using the verbs %T and %p, special formatting considerations apply for operands that implement certain interfaces. In order of application:

  1. If the operand is a reflect.Value, the operand is replaced by the concrete value that it holds, and printing continues with the next rule.

  2. If an operand implements the Formatter interface, it will be invoked. Formatter provides fine control of formatting.

  3. If the %v verb is used with the # flag (%#v) and the operand implements the GoStringer interface, that will be invoked.

If the format (which is implicitly %v for Println etc.) is valid for a string (%s %q %v %x %X), the following two rules apply:

  1. If an operand implements the error interface, the Error method will be invoked to convert the object to a string, which will then be formatted as required by the verb (if any).

  2. If an operand implements method String() string, that method will be invoked to convert the object to a string, which will then be formatted as required by the verb (if any).

As %x and %X are valid verbs for a string, I don't see this behavior changing without introducing compatibility issues.

Let me know if you agree with that reasoning.

@martisch
Copy link
Contributor

@martisch martisch commented Sep 17, 2019

That a type implementing Stringer calls the String method first for %x is working as documented and wont likely change due to keeping backwards compatibility. It has also come up and was discussed in other reports e.g. #21535.

If you want to control the formatting for different verbs. e.g. %s and %x I think implementing the fmt.Formatter interface can be used to let the verb %x print the type as a hex number without modifying other code. Would that work for the use case here?

@wti
Copy link
Author

@wti wti commented Sep 18, 2019

Thank you for the quick replies, the links to docs and bug, and the suggestion to implement Format(..). Sorry I didn't find any of that myself.

Some feedback in case it helps others...

I noticed that the Stringer()/Error() approach is not used for struct members. The docs say when printing struct using %#v, the default format is %#x for int types, and indeed hex is printed for members as I would hope.

https://play.golang.org/p/7pkwiNn5Bc4

type mi uint64

type si struct {
	A mi
}

func (m mi) String() string {
	return "A"
}

// add `(a mi)` to declare on mi, to see verb is `v`
func Format(s fmt.State, verb rune) {
	fmt.Printf("Format mi %c\n", verb);
	s.Write([]byte("hi"))
}

func main() {
	a := mi(0x12)
	s := si{a}
	fmt.Printf("%%#x %#x\n%%#v %#v\n", a, s)
}

// Output (0x41 is hex 'A')
%#x 0x41
%#v main.si{A:0x12}

I suppose v is the verb of record there, and I don't want to change that behavior. When I implement Format on the struct member types and v-format the struct (via fmt.Fprintf("%v", s)), the verb passed to Format for the struct member type is v, not x.

Otherwise, implementing one verb in Format(..) seems to involve re-building the format string for all others, which seems error-prone - particularly if I'm being passed a v instead of a x. hmm. I would have preferred an API that let me opt in for one verb without having to do all.

I don't have a good answer for backwards compatibility issue. Aside from existing code base, 'x' is too common a format verb to deprecate without adding confusion. A new verb for the old behavior could provide a migration path, but with awkward trade-off's.

As for avoiding this: I can search docs for Stringer and fmt for good places to highlight this issue, or find ways to promote Formatter. E.g., with error re-working afoot, I imagine people implementing errors as numeric codes would print out the hex message instead of the hex code. Oops!

I'm still not convinced it's the right thing to adopt String() and Error() for hex output, but I don't see a better path.

Thanks for your quickness and patience with something I should have found in the docs myself.

@wti
Copy link
Author

@wti wti commented Oct 13, 2019

Sorry, I was fishing for feedback on three points:

  1. Whether I should make proposals to update docs for Format or Stringer (or ??) to document the interaction with fmt.

Whether I should write a bug or doc request against fmt...

  1. ...for the behavior of passing the parent format verb v to the Format string when printing a numeric member type (where the verb should be x).

  2. ... for Format's difficulty of implementing all format verbs (instead of only selectively implementing some verbs).

I recognize it's likely best to just try any/all of these and get feedback on the concrete proposal.
I'll close this bug soon absent feedback.

@martisch
Copy link
Contributor

@martisch martisch commented Oct 15, 2019

  1. I think is documented at https://golang.org/pkg/fmt/:

If the format (which is implicitly %v for Println etc.) is valid for a string (%s %q %v %x %X), the following two rules apply:
....
If an operand implements method String() string, that method will be invoked to convert the object to a string, which will then be formatted as required by the verb (if any).

If wording can be improved/clarified you can send a CL to do so. I do not think this would need an extra proposal. Details can be discussed on the CL.

Filling bugs/feature proposals for 2. and 3. sounds good to me. It would be good to describe the problem again in the proposal that is meant to be solved.

@wti
Copy link
Author

@wti wti commented Oct 18, 2019

Thanks for the feedback!

@wti wti closed this Oct 18, 2019
@golang golang locked and limited conversation to collaborators Oct 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.