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

cmd/compile: String() on structs containing empty structs has changed #19246

Closed
heschik opened this issue Feb 22, 2017 · 4 comments

Comments

Projects
None yet
5 participants
@heschik
Copy link
Contributor

commented Feb 22, 2017

Prior to 8958d8c (cmd/compile: skip convT2E for empty structs), the below test didn't call B.String(). After that commit it does, breaking a test that compared the result. I think this is actually a bug fix, but since that commit was supposed to be a pure performance improvement, some investigation is probably warranted.

package test

import (
	"fmt"
	"testing"
)

type A struct {
	Cs []*C
}

func (a *A) String() string {
	return fmt.Sprintf("%+v", *a)
}

type B struct{}

func (b *B) String() string {
	return fmt.Sprintf("B.STRING WAS CALLED %+v", *b)
}

type C struct {
	Name string
	B    *B
}

func (c *C) String() string {
	return fmt.Sprintf("%+v", *c)
}

func TestString(t *testing.T) {
	a := A{
		Cs: []*C{{
			Name: "ae1",
		}},
	}
	want := "{Cs:[{Name:ae1 B:<nil>}]}"

	if g, w := a.String(), want; g != w {
		t.Errorf("got %q want %q", g, w)
	}
}
@heschik

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2017

@dsnet dsnet added the NeedsFix label Feb 22, 2017

@dsnet dsnet added this to the Go1.9 milestone Feb 22, 2017

@dsnet dsnet added NeedsInvestigation and removed NeedsFix labels Feb 23, 2017

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2017

In fact, the String method is called both on tip and on Go 1.8 (and before). On Go 1.8, it panics with nil pointer (*b) and the fmt package recovers it and prints "<nil>". On tip, it doesn't panic, because b is not nil (but should be). A simple repro:

package main

type B struct{}

//go:noinline
func f(i interface{}) {}

func main() {
        var b *B
        f(*b)
}

On Go 1.8, this code panics on f(*b). On tip, it doesn't.

@josharian

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2017

Thanks for the bug report! And thanks, @cherrymui, for the minimization. Fix coming soon.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 23, 2017

CL https://golang.org/cl/37395 mentions this issue.

@gopherbot gopherbot closed this in d9270ec Feb 24, 2017

@golang golang locked and limited conversation to collaborators Feb 24, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.