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: slices: add Of function #61213

Open
FGasper opened this issue Jul 6, 2023 · 13 comments
Open

proposal: slices: add Of function #61213

FGasper opened this issue Jul 6, 2023 · 13 comments
Labels
Milestone

Comments

@FGasper
Copy link

FGasper commented Jul 6, 2023

In #47709 it was proposed to add type inference for slice literals, e.g., foo := []{"bar", "baz"}. That proposal was declined in favour of creating an equivalent function in slices.

This issue proposes a slices.Of(…) method to do that, e.g.:

foo := slices.Of("bar", "baz")

… would be equivalent to:

foo := []string{"bar", "baz"}

The benefit is minimal in a trivial case like the above, but to create a slice of *somepackage.NameThatIsRatherLong it would be helpful.

@gopherbot gopherbot added this to the Proposal milestone Jul 6, 2023
@FGasper FGasper changed the title proposal: affected/package: proposal: slices.Of() Jul 6, 2023
@ianlancetaylor ianlancetaylor changed the title proposal: slices.Of() proposal: slices: add Of function Jul 6, 2023
@ianlancetaylor
Copy link
Contributor

As you say, I can't see anybody using this rather than []string{}. It would help to point to some existing code that would benefit from changing to slices.Of. Thanks.

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 6, 2023
@zephyrtronium
Copy link
Contributor

The benefit is minimal in a trivial case like the above, but to create a slice of *somepackage.NameThatIsRatherLong it would be helpful.

I would argue the opposite: you would have to write

slices.Of(&somepackage.NameThatIsRatherLong{...}, &somepackage.NameThatIsRatherLong{...})

when you could just write

[]*somepackage.NameThatIsRatherLong{{...}, {...}}

Compared to writing a slice literal, slices.Of would only ever have the advantage of inferring the element type from arguments. That seems only useful when the element type is the default type of untyped constants, as in the original example with strings, or when the element type cannot be named e.g. due to being unexported. Maybe it could save some typing when combined with type inference on assignments from functions as well.

@FGasper
Copy link
Author

FGasper commented Jul 7, 2023

Maybe it could save some typing when combined with type inference on assignments from functions as well.

Right, something like:

foo := returnsInstanceOfUglyLongName(alpha)
bar := returnsInstanceOfUglyLongName(beta)

metaVars := slices.Of(foo, bar)

@FGasper
Copy link
Author

FGasper commented Jul 11, 2023

@Nasfame: I’m envisioning its being as simple as:

func Of[T any](pieces ...T) []T {
	return pieces
}

So nos here would be of type []newInt.

As to performance: I don’t know Go internals well enough to speculate competently, but I would think the function call to be less performant than a slice literal. Of course, if Of() proves popular enough, that may speak in favour of making it a language-native feature.

@ianlancetaylor
Copy link
Contributor

Note that that suggested implementation means that

    a := []int{1, 2, 3}
    b := slices.Of(a...)
    a[0] = 4
    fmt.Println(b[0])

will print 4. Anybody have an opinion on whether that will be confusing?

@bcmills
Copy link
Contributor

bcmills commented Jul 11, 2023

I think that would be confusing. I would implement it as:

func Of[T any](pieces ...T) []T {
	return append([]T(nil), pieces...)
}

and trust (or improve) the compiler to eliminate the intermediate argument slice when it isn't needed.

[Edited to add the explicitly-typed nil that is required today.]

@FGasper
Copy link
Author

FGasper commented Jul 11, 2023

I’m probably just missing something, but that seems to contradict the specification:

… the value passed is a new slice of type []T with a new underlying array whose successive elements are the actual arguments …

If b has a new array, why would assigning to a[0] affect b?

@ianlancetaylor
Copy link
Contributor

@FGasper Read a few lines down.

If the final argument is assignable to a slice type []T and is followed by ..., it is passed unchanged as the value for a ...T parameter. In this case no new slice is created.

@FGasper
Copy link
Author

FGasper commented Jul 13, 2023

Here’s a use case for this that I ran into just now:

myMap := map[string]any{}
if condition {
    myMap["foo"] = "bar"
} else {
    myMap["foo"] = "qux"
}

// This function takes a slice of maps; in our case, we only pass one.
DoThingWithDicts( ctx, slices.Of(myMap), "something else" )

Here, Of() alleviates an unergonomic duplication of map[string]any.

@FGasper
Copy link
Author

FGasper commented Aug 10, 2023

To sum up this proposal’s rationale: for the same reason why we all enjoy the freedom to write

foo := "bar"

… rather than

var foo string = bar

… it would be useful for Go to apply type inference to slice literals, e.g., foo := []{"bar", "baz"}. A proposal to implement this was declined, with a proposed alternative from @ianlancetaylor of creating slices.Of to achieve something comparable. (Arguably better, actually, since slice literals don’t currently allow unrolling, e.g., foo := []{"bar", others...}.

This issue basically just formalizes that slices.Of proposal.

(I should note that the original slices.Of proposal was pretty popular, with 9 thumbs-up and 0 thumbs-down.)

@seankhliao seankhliao removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

7 participants
@FGasper @zephyrtronium @ianlancetaylor @bcmills @gopherbot @seankhliao and others