storage_test: compilation failure with go 1.11

[decathorpe]

$GOPATH/src/github.com/cznic/ql/storage_test.go:423: dbg call has arguments but no formatting directives

[cznic]

The dbg call does exist at the line you reported. I can easily remove it, but I believe the compilation failure in go1.11 is most probably a vet bug. dbg is not a well know function taking formatting string in its first arument, like fmt.Printf, for example.

I don't see any ql bug here. Please report this problem directly at the Go issue tracker. Thank you.

[cryptix]

I think this is unfortunate false-positive but I fear a lot of people might come here for this.

Would dbg("Error %s", err) work as well?

[cznic]

Would dbg("Error %s", err) work as well?

Sure it will. But the language specification does not allow the Go compiler to reject a valid program, AFAICT. At least I recall I've been told that while discussing some other issue I forgot already about.

Anyway, if I forget a valid format directive in a call to fmt.Printf, log.Printf etc, I'm actually quite happy vet catches the problem. The problem is that there's no problem in a call to dbg with no formatting arguments. The function is this:

func dbg(s string, va ...interface{}) {
    if s == "" {
        s = strings.Repeat("%v ", len(va))
    }
    _, fn, fl, _ := runtime.Caller(1)
    fmt.Printf("dbg %s:%d: ", path.Base(fn), fl)
    fmt.Printf(s, va...)
    fmt.Println()
}

src

vet has no bussines in this case. Also, not every function with signature func(string, ...interface{}) is a printf-like function. That's why I believe this should be fixed in vet.

Well, and if the compiler guys reject that, I will be sad, but I will remove the offending line, of course. But please let's first try to fix vet.

[decathorpe]

Thanks for looking into this - I reported this as a probable error in go vet here: https://github.com/golang/go/issues/26486

[ianlancetaylor]

@cznic I came here from the Go project issue. The dbg function does indeed pass its arguments to fmt.Printf, so the call to dbg("", err) does look wrong to me. Why does it seem right to you?

[cznic]

@ianlancetaylor The code seen in https://github.com/cznic/ql/issues/203#issuecomment-406381754 handles specially the first argument of dbg when it's equal to "" in such a way that it becomes a valid format string of fmt.Printf.

It's not nice or idiomatic code, but I use the dbg a) for years b) as a quick and dirty debuging aid. In those years, I've typed the call probably some thousands of times. One will soon realize that automatically creating the right "default" format string is just making debugging a bit more comfortable.

tl;dr: The format string finally passed to fmt.Printf when the first argument to dbg is "" is a valid format string. I don't think vet is right when making such code fail the compilation and thus disabling go test to execute. Maybe I'm missing something, please let me know in that case.

It still holds that I'll fix it on my side if the decision is to keep vet "deep" inspecting user code w/o understanding that the "bad" case is actually handled correctly.

[ianlancetaylor]

Ah, I see, sorry for missing that.

I think that on balance vet is making the right choice here but let's discuss that over on the Go project issue.

[cznic]

This should be now fixed at tip: https://go-review.googlesource.com/c/go/+/125039

[decathorpe]

I can confirm that go 1.11beta3 contains the fix. Thanks for your help!