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

append(s) [append allows just a single argument, does not require anything to be appended] #48175

Closed
massar opened this issue Sep 3, 2021 · 3 comments

Comments

@massar
Copy link

massar commented Sep 3, 2021

Suggested Label: #LateNightCoderDefence

TLDR: No compiler warning for "s = append(s)" which is a useless NOOP, would be good to have.

Hi,

A question, because this likely has been discussed before, and wanting to avoid other LateNightCoder's to run into this silly mistake.

Looking at the current branch (though this prototype likely never changed since the beginning):
We got in https://github.com/golang/go/blob/master/src/builtin/builtin.go#L134 :

func append(slice []Type, elems ...Type) []Type

which means that if you use:
`
var s []string

s = append(s)
`

The compiler is happy and you won't notice your mistake why nothing is being added (imagine a big loop around it where you are trying to add items to the slice) till you finally realize that the actual thing being to-be-added is missing [fortunately did not take more than 5 mins this time]

If the compiler is sneaky it could just optimize the above away if we are lucky, but it is very likely the implementer did not mean what is written.

Thus the big Q to avoid LateNightNightCoder problems:

Why not define append as:

A) Prototype with fixed arg

func append(slice []Type, elem Type, elems ...Type) []Type

That would indeed require elem to be processed outside the loop in append thus less good for the code, but would require at least one argument and make the compiler complain because the prototype requires at least one argument.

B) Prototype vararg annotation which indicates a requirement for minimal number of arguments

Another version would be where one could annotate a "minimum required varargs".
But I don't think that is very go to do pragmas or other such things.

Though, this can also be useful for other situations where one would like to have at least 20 params or something else silly (though pass them then as an array? :) )

C) Any other much better other option?

?

What would be a reasonable approach to avoid others to fall into this pitfall? :)

@seankhliao
Copy link
Member

staticcheck (and by proxy gopls) warns about this

@randall77
Copy link
Contributor

Dup of #30040

@massar
Copy link
Author

massar commented Sep 3, 2021

@randall77 thanks for finding it, it is indeed a dupe of #30040 which has a lot more detail, and also simply shows ..... people run into this silently ;)

@golang golang locked and limited conversation to collaborators Sep 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants