References to Interfaces maybe shouldn't be pointers?

flyinprogrammer commented 6 months ago

What I expected:

Given a class that extends an abstract class, like Person:

When you reference a Person in another class as nullable, like with Bug:

Then, we should generate a reference to the Person interface because interfaces themselves are pointers and nil by default.

What I have observed:

Then, we generate a reference to a pointer of Person:

https://github.com/apple/pkl-go/blob/45411666b8b887846c00f4e917a1dff8ac539d54/codegen/snippet-tests/output/bugholder/Bug.pkl.go#L10-L12

What is the problem?

When we use a Bug and access the Owner, we have to dereference the value to access the data and create named constructors to use our structs:

https://github.com/flyinprogrammer/pkl-go-bananas/blob/7210f20ca30593f9d0ebad3665bcec1ebed6a128/main.go#L9-L28

func NewPerson() earth.Person {
    return &earth.PersonImpl{
        FirstName: "Alan",
        LastName:  "Scherger",
    }
}
func main() {
    alanPerson := NewPerson()

    atlasMoth := earth.Bug{
        Name:  "Addison",
        Owner: &alanPerson,
    }

    fmt.Println("Type of alanPerson: ", reflect.TypeOf(alanPerson))
    fmt.Println("Type of atlasMoth: ", reflect.TypeOf(atlasMoth))
    me := *atlasMoth.Owner
    fmt.Println("Type of me: ", reflect.TypeOf(me))
    fmt.Println("atlasMoth Owner GetFirstName: ", (*atlasMoth.Owner).GetFirstName())
    fmt.Println("me PersonImpl FirstName: ", me.(*earth.PersonImpl).FirstName)

}

This is very awkward.

Maybe we can add logic to stop adding a pointer to interfaces. That could be super hard. I don't know 😢 but I can come back to this later if no one else does; I don't have the time at this moment.

bioball commented 6 months ago

Makes sense. We can probably add an option to prefer non-pointers.

thomaspurchas commented 4 months ago

@flyinprogrammer does #83 address your concerns here?

flyinprogrammer commented 2 months ago

@thomaspurchas I believe so! I'm more than happy to validate, too, post-merge!

apple / pkl-go