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

proposal: Go 2: relax the requirements for embedded types #24062

Closed
dotaheor opened this Issue Feb 23, 2018 · 10 comments

Comments

Projects
None yet
4 participants
@dotaheor
Copy link

dotaheor commented Feb 23, 2018

The Problem

The current requirement rules for embedded types is some too unnecessarily restrict,
which makes it is hard to describe the rules clear. The current description in Go spec is not very clear.
Here, I try to describe the rule in one line

The embedded types mustn't be defined pointer types, unnamed types
(except unnamed types whose base types are non-interface and non-poiner defined types).

Though the description is only one line, it is some complex.

The Proposal

I think it is unnecessary to forbid embedding defined pointer types and unnamed pointer types whose base types are defined types. If these types are allowed, then the above description can be simplified to

The embedded types mustn't be unnamed types,
except unnamed pointer types whose base types are named types.

[edit] or

The embedded types must be named types,
or unnamed pointer types whose base types are named types.

@gopherbot gopherbot added this to the Proposal milestone Feb 23, 2018

@gopherbot gopherbot added the Proposal label Feb 23, 2018

@davecheney

This comment has been minimized.

Copy link
Contributor

davecheney commented Feb 23, 2018

Can you please give an example of the two types of forbidden embedded types, and then show what the example would look like if this proposal was accepted.

@dotaheor

This comment has been minimized.

Copy link
Author

dotaheor commented Feb 23, 2018

Now, these types can't be embedded,

type PInt *int
type PPIntAlias = *PInt
type I interface{m()}

type _ struct {
	PInt
	*I
	PPIntAlias
}

I feel there are not harms to allow them to be embedded. And I think whether or not a type is capable to be embedded should be determined by whether or not the type can provide a name.

@davecheney

This comment has been minimized.

Copy link
Contributor

davecheney commented Feb 23, 2018

@tklauser tklauser changed the title proposal: relex the requirements for embedded types proposal: relax the requirements for embedded types Feb 23, 2018

@dotaheor

This comment has been minimized.

Copy link
Author

dotaheor commented Feb 23, 2018

Then what is the use case for embedding int, *string and type A = []bool?
At present, embedding these types is legal.

type A = []bool
type B map[string]string
type C = *int
type _ struct {
	int
	*string
	A
	*B
	C
}

I think, at least, it is not harms to embed them.

@ianlancetaylor ianlancetaylor changed the title proposal: relax the requirements for embedded types proposal: Go 2: relax the requirements for embedded types Feb 23, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Feb 23, 2018

I do not think that your proposed revision is simpler. I don't know what a "one-level pointer type" is.

The point of an embedded field is to add the embedded field's methods to the struct type's method set. If you don't care about method sets, there is no reason to use an embedded field.

Given that, the restrictions on which types are permitted are to clarify for the user how the method set is used.

Using *InterfaceType where InterfaceType is an interface type is forbidden because although InterfaceType presumably has methods, *InterfaceType does not. Interface types differ from other types in whether the method set flows through to a pointer type; this is not going to change. We forbid using *InterfaceType as an embedded field because 1) there is no reason to write such a thing; 2) if people do it anyhow they will almost certainly be confused by its behavior, so better to give an error at compile time.

We forbid embedding a pointer type because pointer types may not have methods. If we permit embedding a pointer type, we will have to decide how to handle the methods of the pointed-to type. Different people will have different opinions about how that should work, and it's never useful because you can always embed *T instead. Forbidding embedding a named pointer type avoids confusion without loss of functionality.

We permit embedding types like int because, even though that is useless, it is not confusing.

@dotaheor

This comment has been minimized.

Copy link
Author

dotaheor commented Feb 23, 2018

@ianlancetaylor "one-level pointer type" means the pointer types whose base types are not pointer types. I just found the terminology is unnecessary, so I removed it from the first comment.

Yes, the relaxed rules have not impact for using Go. It is just that we can explain the rules more easily for new gophers.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Feb 23, 2018

On the other hand, the current rules permit the compiler to give better error messages.

@dotaheor

This comment has been minimized.

Copy link
Author

dotaheor commented Feb 24, 2018

I would argue that the proposal will make the messages even better and clear.
And I think the error message of gc for bad embedding is not perfect.
For example, two of the following embedding are bad, but they are all pointer types.

type PInt *int
type PPIntAlias = *PInt
type B map[string]string
type C = *int

type _ struct {
	PInt       // error: embedded type cannot be a pointer
	PPIntAlias // error: embedded type cannot be a pointer
	*B
	C
}
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Apr 24, 2018

The main reason given for this change seems to be to slightly simplify the spec. That doesn't seem like a sufficient reason for reducing the quality of compiler error messages by introducing more cases that are unlikely to act as people new to the language would expect.

@dotaheor

This comment has been minimized.

Copy link
Author

dotaheor commented Apr 25, 2018

ok.

But I still think the current type embedding requirements bring much difficulties for new gophers to understand what types can be embedded and what can't.

(And I think this proposal is Go 1 compatible.)

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