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

Invalid Go code when non-nil pointers to built-in types. #4

Open
dmitshur opened this issue Jan 27, 2014 · 4 comments
Open

Invalid Go code when non-nil pointers to built-in types. #4

dmitshur opened this issue Jan 27, 2014 · 4 comments

Comments

@dmitshur
Copy link
Collaborator

In Go, it's valid to create pointers to literals of named types, e.g, &MyStruct{5}, but not for built-in types (incidentally, if someone could explain the motivation for this difference in behavior, I'd love to know please).

Currently, go-goon will naively output things like:

(MyStruct)(MyStruct{
    Field1: (*string)(&"string pointer"),
    Field2: (*AnotherStruct)(&AnotherStruct{
        Name: (string)("name"),
    }),
})

Which isn't valid Go, which goes against goon spec.

One possible solution is to rely on helper funcs that convert built-in literals to pointers of said built-in literals.

An example of this being done (manually edited) can be seen here.

The formula for such helper funcs (which can be put into an importable package) I've used so far:

func NewString(s string) *string {
        return &s
}

func NewInt(i int) *int {
        return &i
}

Is that a good naming scheme? Would it conflict with anything commonly used? If not, perhaps this can be used. Unless the helper funcs are imported, the goon will still be invalid Go code, but at least it's more easily fixable compared to current output which is completely invalid (unless Go spec changes to allow pointers to built-in literals... why can't that be done again?).

@dmitshur
Copy link
Collaborator Author

dmitshur commented Nov 7, 2015

I think I want to go with using those NewString and similar methods.

The only question is where they should live. I'd like some sort of canonical location.

Related note, see davecgh/go-spew#37 (comment).

@dmitshur
Copy link
Collaborator Author

dmitshur commented Apr 17, 2017

@fortytw2 has shared package github.com/AlekSi/pointer with me, which is relevant to this issue, and we should consider it.

@dmitshur
Copy link
Collaborator Author

dmitshur commented Feb 8, 2018

@AshKash made a very interesting suggestion for another solution in #12:

This is not valid Go. A trick to make it work is:

(Y)(Y{
	Y1: (*string)(&[]string{"str"}[0]),
	Y2: (*int32)(&[]int32{42}[0]),
})

I hadn't thought of it before now. It's definitely worth considering.

@dmitshur
Copy link
Collaborator Author

dmitshur commented Feb 8, 2018

I've thought about it, and I'm ready to go for it. Even though the syntax is more verbose, it has the benefits of being correct and resolving a bug. Compared to the previous idea (which I wasn't too happy with), it's better because it doesn't require external packages.

I think we can take the first step of resolving this bug with @AshKash's idea (thank you for making it!). Then, we can consider other approaches if they happen to be cleaner.

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

No branches or pull requests

1 participant