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

cmd/vet: mismatch between Printf checks and actual behaviour #27672

Open
dominikh opened this Issue Sep 14, 2018 · 11 comments

Comments

Projects
None yet
5 participants
@dominikh
Member

dominikh commented Sep 14, 2018

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

go version go1.11 linux/amd64

What did you do?

What did you expect to see?

Vet to accept the first Printf call and to flag the second one

What did you see instead?

Vet flags the first call and accepts the second one

@dominikh dominikh added the NeedsFix label Sep 14, 2018

@dominikh dominikh added this to the Go1.12 milestone Sep 14, 2018

@dominikh

This comment has been minimized.

Member

dominikh commented Sep 14, 2018

/cc @mvdan

@dominikh

This comment has been minimized.

Member

dominikh commented Sep 14, 2018

Another false negative:

var x struct{ A *int }
fmt.Printf("%p\n", x)
@dominikh

This comment has been minimized.

Member

dominikh commented Sep 15, 2018

One more false negative:

var x [2]int
fmt.Printf("%p", x)

@dominikh dominikh changed the title from cmd/go: vet handles pointers in Printf arguments incorrectly to cmd/vet: mismatch between Printf checks and actual behaviour Sep 15, 2018

@dominikh

This comment has been minimized.

Member

dominikh commented Sep 15, 2018

Another false positive, presumably because the check is confused by the Error method it can't call. fmt.Printf is fine with that though, as long as the concrete type is a string. https://play.golang.org/p/qAs0Ye8atHL

@mvdan

This comment has been minimized.

Member

mvdan commented Oct 1, 2018

Thanks for the ping - I might have a look at this during the current cycle.

@mvdan mvdan self-assigned this Oct 1, 2018

@gopherbot

This comment has been minimized.

gopherbot commented Nov 6, 2018

Change https://golang.org/cl/147758 mentions this issue: cmd/vet: fix some pointer false positives in printf

@mvdan

This comment has been minimized.

Member

mvdan commented Nov 6, 2018

I've filed an issue about the second case in the original comment, as I think vet is technically doing what fmt documents.

@gopherbot

This comment has been minimized.

gopherbot commented Nov 6, 2018

Change https://golang.org/cl/147997 mentions this issue: cmd/vet: fix printf false negative with nested pointers

gopherbot pushed a commit that referenced this issue Nov 9, 2018

cmd/vet: fix some pointer false positives in printf
fmt's godoc reads:

	For compound objects, the elements are printed using these
	rules, recursively, laid out like this:

		struct:             {field0 field1 ...}
		array, slice:       [elem0 elem1 ...]
		maps:               map[key1:value1 key2:value2 ...]
		pointer to above:   &{}, &[], &map[]

That is, a pointer to a struct, array, slice, or map, can be correctly
printed by fmt if the type pointed to can be printed without issues.

vet was only following this rule for pointers to structs, omitting
arrays, slices, and maps. Fix that, and add tests for all the
combinations.

Updates #27672.

Change-Id: Ie61ebe1fffc594184f7b24d7dbf72d7d5de78309
Reviewed-on: https://go-review.googlesource.com/c/147758
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>

gopherbot pushed a commit that referenced this issue Nov 9, 2018

cmd/vet: fix printf false negative with nested pointers
Pointers to compound objects (structs, slices, arrays, maps) are only
followed by fmt if the pointer is at the top level of an argument. This
is to minimise the chances of fmt running into loops.

However, vet did not follow this rule. It likely doesn't help that fmt
does not document that restriction well, which is being tracked in
 #28625.

Updates #27672.

Change-Id: Ie9bbd9b974eda5ab9a285986d207ef92fca4453e
Reviewed-on: https://go-review.googlesource.com/c/147997
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
@dgryski

This comment has been minimized.

Contributor

dgryski commented Nov 15, 2018

This false positive seems related:

$ cat main.go
package main

import (
	"fmt"
)

type foo struct {
	root *node
}

type node struct {
	value int
}

type foo2 struct {
	root int
}

func main() {
	var t foo
	fmt.Printf("t = %x\n", &t)

	var t2 foo2
	fmt.Printf("t2 = %x\n", &t2)
}
bug $ vet .
/Users/dgryski/go/src/github.com/dgryski/bug/main.go:21:2: Printf format %x has arg &t of wrong type *github.com/dgryski/bug.foo

It seems like vet doesn't know that %x happily prints pointers, but it seem to only know not that for foo, not foo2 which doesn't itself contain a pointer.

(If it's not related, I can open another bug)

@mvdan

This comment has been minimized.

Member

mvdan commented Nov 15, 2018

I don't intend to work more on vet during this cycle, so I'm unassigning myself in case someone else wants to continue the work. Right now, only the two bugs in the original post are fixed; the other four aren't fixed yet, assuming they're not duplicates.

@mvdan mvdan removed their assignment Nov 15, 2018

@dgryski

This comment has been minimized.

Contributor

dgryski commented Nov 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment