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: spec: permit eliding the type of struct fields in nested composite literals #21496

Open
neild opened this issue Aug 17, 2017 · 16 comments
Labels
LanguageChange Suggested changes to the Go language LanguageChangeReview Discussed by language change review committee Proposal
Milestone

Comments

@neild
Copy link
Contributor

neild commented Aug 17, 2017

Within a composite literal of array, slice, or map type T, elements or map keys that are themselves composite literals may elide the respective literal type if it is identical to the element or key type of T. Similarly, elements or keys that are addresses of composite literals may elide the &T when the element or key type is *T.

When the type of a nested composite literal may derived from its context, that type may be elided in all cases except when the composite literal is the value of a struct field.

type T struct{ V int }

var _ = map[T]T{{1}: {1}}      // type elided from map key and element
var _ = []T{{1}}               // type elided from slice element
var _ = [...]T{{1}}            // type elided from array element
var _ = struct{ F T }{F: T{1}} // type is required

I propose that elision be permitted in this case as well.

var _ = struct{ F T }{F: {1}}
var _ = struct{ F *T }{F: {1}}

I submit that the arguments in favor are the same as for eliding the type in other contexts, as well as conceptually simplifying the language--it is simpler to say that types may always be elided in nested literals (when the type can be derived from context) than to say that types may be elided in all but one case.

Proposed spec change:

Within a composite literal of array, slice, struct, or map type T, ...

This proposal is a subset of #12854, which proposes elision of types in all composite literals, not just nested ones.

@neild neild added the Proposal label Aug 17, 2017
@jimmyfrasche
Copy link
Member

Not an argument for or against, but personally I always forget that this case is not allowed because it feels so similar to the cases where I can elide the type.

@griesemer griesemer changed the title Permit eliding the type of struct fields in nested composite literals Proposal: Permit eliding the type of struct fields in nested composite literals Aug 17, 2017
@gopherbot gopherbot added this to the Proposal milestone Aug 17, 2017
@griesemer griesemer added the LanguageChange Suggested changes to the Go language label Aug 17, 2017
@griesemer
Copy link
Contributor

Also not an argument for or against, just noting that this was discussed when we discussed type elision for array/slice/map literals. At that time we didn't have any experience with this form of type elision and we decided to err on the side of caution: There was a sentiment that type elision in struct literals may be more confusing (read: lead to less readable code) than elsewhere due to the possibly different types of struct fields. In contrast, for arrays, slices, and maps, all the (keys and) elements are of the same type.

@neild
Copy link
Contributor Author

neild commented Aug 17, 2017

I'll have to dig up specific examples, but in my experience the lack of elision often leads to stutter in code:

req := service.AddRequest{
  A: 1,
  B: 3,
  // The Options field contains a value of type Options. Quelle surprise!
  Options: service.Options{
    SurrealNumericRegime: true,
  },
}

This is particularly heinous in massive, nested protocol buffer definitions, although I'm sympathetic to the argument that the fault lies in the massive, nested definitions rather than the language literal syntax. :)

@rsc
Copy link
Contributor

rsc commented Aug 21, 2017

When the type of a nested composite literal may derived from its context, that type may be elided in all cases except when the composite literal is the value of a struct field.

Actually the rule is that when the type of all the elided things is the same, the type may be elided. In a struct (in general), fields may have different types, so it's hard to see what the thing being elided is.

We were concerned with structs in particular because interface fields would still need the type, for example if you have an R io.Reader, you have to write R: &bytes.Buffer{} not R: {}, so you can't just wholesale elide them. And in general it's hard to see what type you have. The stutter in other examples is true too. The balance fell the current way back when we considered it last time.

One thing I realized over the weekend is that this has implications for API design. In os/exec we have a SysProcAttr *syscall.ProcAttr field, and the intent was that you have to import "syscall" to fill one out, so that it is explicit you are writing syscall-specific code. If we allow elision, then you can just write SysProcAttr: {Foo: 1} and not realize that the inner struct literal is syscall-specific: there's no syscall import in your program.

Just things to think about, not an argument for or against.

@rsc rsc added the v2 An incompatible library change label Aug 21, 2017
@rsc rsc changed the title Proposal: Permit eliding the type of struct fields in nested composite literals proposal: spec: permit eliding the type of struct fields in nested composite literals Aug 21, 2017
@neild
Copy link
Contributor Author

neild commented Aug 21, 2017

I'm not certain that it quite holds that the type of all the elided things is the same--maps permit elision of both keys and values, which are not necessarily the same type. Or if we say that all the keys of a map have the same value, then it is equally true that all the values of field F of a struct have the same value.

I also don't entirely buy the point about interface fields; you can't elide the type from non-struct types in this case either. It's true that most implementations of an interface are usually struct types, but that's not necessarily the case. And, of course, permitting or forbidding elision of struct field types in other cases doesn't change this case at all, since you need to include the type either way.

@jimmyfrasche
Copy link
Member

One thing to note is that the elisions done currently can always be done without impeding readability.

This (and the others in #12854) would be elision(s) that could help or hinder readability depending on the context.

gofmt -s can always perform the current elisions.

This would have to be a choice. As such, gofmt -s could never perform this elision, though it would be a valid simplification.

Being a choice, different codebases, or different sections in the same codebase could make the choice differently. However, this is already true of keyed vs. unkeyed fields in a struct. While keyed literals are generally preferred, unkeyed literals can make equal sense in some situations so there's no absolute rule.

I would prefer the option to make this choice when it would help readability, personally.

If a type is elided in such a way as to hinder readability, or in an inconsistent manner, that's a matter for code review and linting.

@neild
Copy link
Contributor Author

neild commented Sep 8, 2017

One thing to note is that the elisions done currently can always be done without impeding readability.

This is a subjective opinion, not a statement of objective fact.

_= T{
  {1, 2, 3, 4}: {"Value"},
}

Is the type elision in this example aiding or impeding readability? I would say that the answer depends on the context of the code, and that opinions will still differ. I do not believe that it is obviously correct that gofmt -s can always perform the current elisions without impeding readability.

Note that the above example is valid under the current elision rules:
https://play.golang.org/p/ccZJAVqV0M

@drscre
Copy link

drscre commented Jul 18, 2018

Will this proposal allow me to use anonymous nested structs?

a := struct {
  A int
  B struct {
    C int
  }
} { 
  A: 10,
  B: {
    C: 10,
  },
}

@owais
Copy link

owais commented Jun 28, 2019

@drscre I think it should as IMO it is probably the case that would benefit the most from this proposal. In fact, I think it'd still be a huge win if the proposal was accepted only for the anonymous struct case. I find myself writing nested structs anonymous structs from time to time and am forced to always break them down into multiple independent structs that are the composed together. I agree that is better for most cases but in some cases it feels unnecessary.

For example, recently I wrote something like this but only a a lot more field.

type SubConfigA struct {
	FirstField  string
	SecondField string
}

type SubConfigB struct {
	FieldA string
	FieldB string
}

type Config struct {
	F1 int
	F2 int
	F3 int

	SubA SubConfigA
	SubB SubConfigB
}

With lots of nested fields and lots of nested structs, it would have been nicer to be able to write the whole thing as a single type as it is very much self-contained.

type config struct {
	F1 int
	F2 int
	F3 int

	SubConfA struct {
		FirstField  string
		SecondField string
	}

	SubConfB struct {
		FieldA string
		FieldB string
	}
}

and then be able to use it to define a defualt config instance without having to re-type the entire definition again or split the struct definition into smaller pieces.

defaultCfg := Config{
	F1: "f1",
	SubConfA: {
		FirstField: "first",
	},
	SubConfB: {
		FieldB: "bee",
	},
}

@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 16, 2019
@gopherbot gopherbot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 3, 2019
@natefinch
Copy link
Contributor

This would be suuuuper useful for configuration structs. When I configure an application, I usually write a config struct like this:

type Config struct {
	Runtime struct {
		Environment string
		Debug       bool
		Admins      []string
		Hostname    string
	}

	GRPC struct {
		Enabled  bool
		Port     int
		Services []string
	}
}

This makes the acceptable values really easy to read.. the anonymous structs just act as namespaces when configuring, so your config file looks something like this:

Runtime:
    Environment: dev
    Debug: true
GRPC:
    Enabled: true

(etc)

But then if you want to fill it out in actual go code, it's kind of awful:

c := Config{
	Runtime: struct {
		Environment string
		Debug       bool
		Admins      []string
		Hostname    string
	}{
		Environment: "dev",
		Debug: true,
 	},
	GRPC: struct  {
		Enabled  bool
		Port     int
		Services []string
	}{
		Enabled: true,
	},
}

The only way around it, would be to make Runtime and GRPC named types, but that then makes it a lot harder to read the type declaration (and thus what actual values are available to be set):

type Runtime struct {
	Environment string
	Debug       bool
	Admins      []string
	Hostname    string
}
type GRPC struct {
	Enabled  bool
	Port     int
	Services []string
}

type Config struct {
	Runtime Runtime
	GRPC GRPC
}

Not only does this make it harder to see what you can set in the Config struct, it also "exposes" the structs internal to Config to the external world, even though they really only exist to namespace values inside the Config struct.

And the annoying thing is that the compiler knows what every type is. Yes, if there's an interface you'd still have to specify, but the compiler would tell you when you forgot to specify that. I don't think "you'd still have to specify a type in one ambiguous case" is a reason not to allow inference the rest of the time.

c := Config{
	Runtime: {
		Environment: "dev",
		Debug: true,
 	},
	GRPC: {
		Enabled: true,
	},
}

With that change, the struct literal becomes almost a perfect duplicate of the type declaration.

Please, let's allow type elision here. It would make a lot of code easier to read, easier to write, and easier to maintain.

@natefinch
Copy link
Contributor

This is labelled go2 but it seems to be backwards compatible. Is there something that makes.it not possible to do in go1?

@networkimprov
Copy link

Go 2 shipped last week.

https://blog.golang.org/go2-here-we-come

@ianlancetaylor
Copy link
Contributor

@natefinch We label all language changes as Go2.

@jba

This comment was marked as off-topic.

@jimmyfrasche
Copy link
Member

@jba I think you want #12854

@ianlancetaylor ianlancetaylor removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 22, 2023
@ConcurrentCrab
Copy link

Hi, I am not sure about the status of this issue, I hope it's still under consideration.

I just wanted to add an example that I didn't see mentioned above, showing that this behavior in practice even causes the rule allowing elision of other types to become useless:

var dataRotate = []struct {
	a, b []int
	c    int
}{
	{
		[]int{1, 2, 3, 4, 5, 6},
		[]int{5, 6, 1, 2, 3, 4},
		2,
	},
	{
		[]int{1, 2, 3, 4, 5, 6, 7},
		[]int{5, 6, 7, 1, 2, 3, 4},
		3,
	},
	{
		[]int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20},
		[]int{14, 15, 16, 17, 18, 19, 20, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13},
		7,
	},
	[...]
}

This is often how table-driven test data for real-life functions look like. This pattern is used often and is recommended by many for the appropriate situations: https://dave.cheney.net/2013/06/09/writing-table-driven-tests-in-go

(Obviously I don't mean to imply that a project member recommending this makes it "official" or whatever, just that it's a very common pattern and its ergonomics should be taken into consideration).

@ianlancetaylor ianlancetaylor added LanguageChangeReview Discussed by language change review committee and removed v2 An incompatible library change labels Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LanguageChange Suggested changes to the Go language LanguageChangeReview Discussed by language change review committee Proposal
Projects
None yet
Development

No branches or pull requests