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: remove compiler error for unused variables #43729

Open
dancojocaru2000 opened this issue Jan 15, 2021 · 54 comments
Open

Proposal: remove compiler error for unused variables #43729

dancojocaru2000 opened this issue Jan 15, 2021 · 54 comments
Labels
Projects
Milestone

Comments

@dancojocaru2000
Copy link

@dancojocaru2000 dancojocaru2000 commented Jan 15, 2021

Seeing as Go is a tiny bit more open minded, going from "GOPATH is the best and only way and it will always be this way absolutely totally" to soon removing it entirely, I'm taking my chances.

A lot of people find no value in this error. Furthermore, many people find it as a hinderance, a bad part of the compiler and the language.

And since the way to get rid of it is simply to _ = variable, which changes nothing in the behavior of the compiled program, this error helps solve nothing related to the running of the program. It's just an annoying reminder of "you forgot to explicitly ignore the variable before you debug and then make actual use of it later anyway like it always happens".

Therefore, according to the FAQ's reasoning: "And if it's not worth fixing, it's not worth mentioning.", Go should stop mentioning this.


And just in case, to address the potential question of "But what about people who find this useful?".

Other languages deal with this easily using linters. That way, people who find an use in this error can keep it, while not impeding both the productivity and joy of many others.

@gopherbot gopherbot added this to the Proposal milestone Jan 15, 2021
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 15, 2021

I think this is effectively a dup of #513, #958, #1085.

@DeedleFake
Copy link

@DeedleFake DeedleFake commented Jan 15, 2021

A lot of people find no value in this error. Furthermore, many people find it as a hinderance, a bad part of the compiler and the language.

I'd like some references, if possible. It can be annoying sometimes, sure, but I quite like this error, actually.

And since the way to get rid of it is simply to _ = variable

This is not the way to get rid of it. The way to get rid of it is to get rid of the variable that you're not using anyways. If you're just debugging and need something removed temporarily, just comment the variable out. If you're not using it anyways, it should generally be pretty easy to do so. The only time I've found it annoying is when it's a variable declared in a condition or it's part of a multi-return value declaration, but it's still only a minor annoyance. Personally, I think that the benefit, namely making sure that you don't have pointless local variables laying around, cluttering up your code, outweighs those annoyances.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Jan 15, 2021

Only somewhat. Rather than just complaining about the error, I'm pointing out that by the very reasoning in #1085 it shouldn't exist.

Many people find this a matter that shouldn't be fixed. Therefore, Go shouldn't complain about it.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Jan 15, 2021

I'd like some references, if possible.

It will be very time consuming to go on the web and look for every individual finding this annoying. Someone on a Reddit post even thought it was a joke and actually looked up Go to see if this is an actual error.

Regardless, since there were three other issues mentioned as potential duplicates and an FAQ section dedicated to this, I will say this is enough to show this this is more than a couple of people who find this a bad part of the langauge.

It can be annoying sometimes, sure, but I quite like this error, actually.

And I dislike the error, so we're equal. :P

Jokes aside, I opened this issue in the hopes that Go moved on from the days of "GOPATH is best and we'll ignore all the negative feedback even if only 10 people will actually like our way". I mean no offense if you're one of the people who finds this useful, but is this good enough when it annoys or even hinders many others?

The way to get rid of it is to get rid of the variable that you're not using anyways.

But what if I'm using it, just not in the way Go expects me to? For example, a common thing I do in other langauges is to declare a variable for the result of a function and then debug until there to see what the function will return. The use of that variable is just to see its value in the debugger, yet Go can't know that it's useful to me.

Or maybe its use is planning ahead, making an idea of what the code will look like. Even if I don't use that variable yet, maybe it will help me to have it there, with the function that creates it called already with the right parameters, but the part that uses it not done yet. If I'm gonna uncomment it later, why comment?

Personally, I think that the benefit, namely making sure that you don't have pointless local variables laying around, cluttering up your code, outweighs those annoyances.

The benefit is a couple less bytes of code. At most a slight visual disturbance in a little piece of the code.

The downside ranges depending on people from mild annoyance to workflow disruption, especially people used to other langauges.

To me, the benefit is insignificant and the downside is quite big.


And, finally, once again, if it benefits you, that's great! Just like the FAQ proposed a tool as a potential (but imo bad) solution for the unused import error, I propose a tool - a linter - that can be used to give you this error back if it is removed from the compiler.

All I'm proposing is that this benefit for some should be kept, but the hinderance to others should be removed.

@mvdan
Copy link
Member

@mvdan mvdan commented Jan 17, 2021

You say this is a big downside for a lot of people, but I can't see any evidence for that statement. For example, if it truly were one of Go's biggest problems, or one that most newcomers struggled with, I would imagine it would show up in developer surveys like https://blog.golang.org/survey2019-results.

Personally, I do see some newcomers surprised by the Go compiler's stance on errors and warnings, but everyone I've worked with gets used to it after a few weeks. There are clear benefits to having this be a compiler error, because it means the check is widespread and hard to ignore. The only downside I see is getting used to it, or the slight annoyance while running unfinished code. But again, I don't see any proof that this is actually a large problem for a lot of Go users.

@beoran
Copy link

@beoran beoran commented Jan 20, 2021

This error has save me many times in cases where I thought I was using a variable, but wasn't. It should stay as it is very useful for this case.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Jan 20, 2021

@beoran As I said, this will be equally as useful as a linter, who would also be able to point it out to you.

It just wouldn't bother those who find this one of the pain points of using Go anymore.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Jan 20, 2021

@mvdan Those developer surveys are not entirely representative of everybody. In particular, they're not representative of people who wanted to learn Go, got screamed at by the compiler and were confused until they found out that you can't have else on a new line and, out of frustration, gave up on learning Go. (Speaking from experience from the first time I tried Go, back when Go modules didn't exist).

At that beginner level, the advantages of Go might not be too clear, and, when faced with an overly restrictive compiler, C++/Python/Node.js do just fine, why bother fighting with a mean compiler and a confusing language?

And since those people generally don't stick around in the Go community to express their disappointment, they're not really heard. They just silently come and go.

Since I didn't hear of that survey until you linked it, it's safe to say that only a certain category of people are represented by it.

I'm not saying that this only affects beginners (on that Reddit post I linked there are a couple of Go users who point out that this is one of the annoyances of using the language). All I'm saying is that quoting such a survey where in all likelihood there's mainly people who like most or all of Go answering isn't exactly the perfect counter argument. "100% of people who like it say they like it"-ish.

@mvdan
Copy link
Member

@mvdan mvdan commented Jan 20, 2021

this will be equally as useful as a linter, who would also be able to point it out to you.

A linter people have to enable will be used by fewer people.

not representative of people who wanted to learn Go

I'm happy to hear about any other way to objectively measure the impact of this compiler error. Personal experience is not really representative. Similarly, online threads tend to attract vocal or unhappy people, so they aren't a good way to get an overall picture.

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jan 20, 2021

Those developer surveys are not entirely representative of everybody.

respectfully this is a mutated version of the “no true Scotsman” argument. Four yearly surveys have not shown that unused variables are a significant cause of complaint. Ignoring this inconvenient fact does not validate your arguments.

I understand that this decision, which was made in the earliest days of go’s development, is not your personal preference, but please understand that of all the things which are unlikely to change in go, this is at the top of the list.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Jan 20, 2021

A linter people have to enable will be used by fewer people.

That's the point? Since not everybody appreciates or wants the error, the logical conclusion is that there will be fewer people.

I'm happy to hear about any other way to objectively measure the impact of this compiler error.

"Does it need an FAQ entry addressing the issue?" would be one. Since enough people complained that it warranted a detailed FAQ entry, and complaints come in even after that, would that not be a good enough measure?

Similarly, online threads tend to attract vocal or unhappy people

So only people happy with the situation should be listened to? Back to "100% of people who like it said they like it"?

so they aren't a good way to get an overall picture.

Not only listening to them, no, of course. But does saying "those opinions aren't valid" not reduce the quality of the overall picture?

I understand that this decision, which was made in the earliest days of go’s development

Wasn't "write all code in GOPATH" the same?

but please understand that of all the things which are unlikely to change in go, this is at the top of the list

Back when Go modules weren't a thing yet, I was figuratively screamed at when I complained about GOPATH being inconvenient. It was the best way to go and it was never gonna change. Now it's soon entirely gone.

Recall that this was my reasoning for bringing this issue up in the first place. Langauges change as they evolve. Maybe if this decision was made early in development, it might mean that it was made without having the community of today to consult with and it might have been a bad decision?


Of course, it's totally a possibility that only like 1% of Go users dislike this. Maybe for people who got used to it it's just annoying on the spot but it's not annoying enough to remember to write it in the survey. I may be a minority and in that case fine, not everybody must fully enjoy the language.

At the same time, quoting just the survey and ignoring what people of online forums say feels a bit like sticking the head in the sand. Which can be acceptable. After all, the core team decides what the language should be. People who like it will use it, people who don't will use something else. C'est la vie. There will appear an issue about it every once in a while, respond with link to FAQ, marked as duplicate, closed, bye.

I just wanted to try to make a case.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Jan 20, 2021

Oh, also, about that survey and related to "other way to objectively measure the impact of this compiler error". I see it's a survey of Go users. Maybe a survey of people who tried Go but abandoned it would also be helpful? "What made you leave?"

@beoran
Copy link

@beoran beoran commented Jan 20, 2021

UseLet me quote purplepharoh from that Reddit thread:

"The thing about GO is that it enforces good practices with compiler errors. For example this could be a warning but most people ignore warnings and often don't fix them, good practice is to remove (or use) unused variables so GO enforces this."

This is an essential feature of Go which ensures consistency and minimum code quality. Code is read more often than it is written and unused variables left over from development are a hindrance to reading. An unused variable definitely is bad practice since it indicates unfinished code or messy code, or worse, a shadowed variable. The latter often happens with err error variables.

For your use case of return values in the debugger, just declare your function with variables for the return, e. g. func Square(in int) (res int).

For planning code, your functions are too complex if you want to add unused variables in there for planning purposes. Besides, in other languages, they have the tendency to stick around when they end up being unneeded after all, or become redundant.

I had similar feelings about Go 10 years ago when I learned Go, but I changed my mind after a few months. While the Go approach of enforcing good practice takes some getting used to, especially when coming from a different language, once you get the hang of it, you'll reap the benefits.

@mvdan
Copy link
Member

@mvdan mvdan commented Jan 20, 2021

It's also worth noting that Go is opinionated by design. If a developer wishing to learn Go has strong convictions and is not willing to keep an open mind, it's likely they would not like Go at all. There are dozens of ways in which Go differs from other mainstream languages, and this is just one amongst many.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Jan 20, 2021

My problem isn't with wanting to ensure readable code. Rather, my problem is with writing code being impeded. What good is making written code look nice if the code won't be written in the first place.

The ideal solution would be to be able to debug the code ignoring non-essential errors like "you didn't use the variable". But since the Go team says that compiling flags for this shouldn't exist and this is all or nothing, I came here to argue for "fine, nothing then".

@beoran That doesn't really fix my usecase since it's not always my function. In particular, functions aren't always pure, and they have side effects. Perhaps I want to debug such a code:

socket, error := connect(address)

Here, Go would complain that socket and error are unused when I want to debug and see their value.

As for planning, writing that line above yet not doing something with the socket yet doesn't sound too complicated.

@mvdan: Understandable, just as Go was opinionated some years ago about GOPATH being the one and only way to go. So then why is GOPATH deprecated?

As I said previously, I don't necessarily mind Go being opinionated, I just think that maybe the current opinion is not the greatest one?

@mvdan
Copy link
Member

@mvdan mvdan commented Jan 20, 2021

Some people will say that the way Go does X is the one true way and it will never change, but that's not really the project's stance. If something has good arguments to be changed, enough to outweigh the disadvantages and the breaking/confusion of existing users, it can happen. Modules brought great things, like reproducible builds and dependency management.

This change, as far as I can tell, just avoids a minor annoyance that few(?) people actually struggle with. With that said, I feel like I'm repeating myself and I don't have anything else to add here, so I'll stop responding for now :)

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 21, 2021

I understand that this decision, which was made in the earliest days of go’s development

Wasn't "write all code in GOPATH" the same?

Just as a historical note, the handling of unused variables does date back to the earliest days of the language. GOPATH was several years later.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Jan 21, 2021
@samlof
Copy link

@samlof samlof commented Jan 21, 2021

This is one of my pain points as well with Go. It doesn't do anything often, and when it does it's frustrating. The unused imports was another equally frustrating, it's good tools do it for you nowadays.

Maybe we need a tool that transparently adds _ = myvar for unused variables when debugging or using go run command? That way we can choose to use it or not, wonder if someone has created it already.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Jan 21, 2021

Unused imports are indeed also annoying to me, but from a different reason, and tools don't help with that.

The reason is autocomplete. Don't add import, autocomplete doesn't know of what function you want to call. Add import, autocomplete complains that the import is not used yet.

But autocomplete is merely an extra convenience, reading the documentation of how to call a certain function before autocomplete will help with the rest isn't too bad.

That's why I only complain about unused variables, which is an error that's actually annoying and at times quite workflow interrupting. And to give not much back as a benefit. I'm not saying I'm an expert top 3 programmers of all time, but I can't recall any time in using another langauge where not using a variable created any problem.

An unused variable definitely is bad practice since it indicates unfinished code or messy code, or worse, a shadowed variable.

In particular, these are some examples, and only the last one is remotely useful (maybe restrict the error only to shadowed variables?). For the other ones, perhaps that shows my ineptitude, but I don't write finished code in one go. I write some unfinished code, run it, see how it behaves and go from there. Not being allowed to run it because it's unfinished is quite a hinderance.

@ingvarm-gr
Copy link

@ingvarm-gr ingvarm-gr commented Jan 25, 2021

@dancojocaru2000 writes:

.... In particular, functions aren't always pure, and they have side effects. Perhaps I want to debug such a code:

socket, error := connect(address)

Here, Go would complain that socket and error are unused when I want to debug and see their value.

I am not sure how useful a "printed representation" of a Go socket structure would be, but purely hypothetically, you could add something like:

log.Printf("DEBUG: connect: socket = %v, error = %v\n", socket, error)

You now have a usage, and you can also see them in the debugger, should you choose to run them under a debugger. You could also write a small variadic shim around that (possibly called debug) and get optional verbose logging later.

@danieldaeschle
Copy link

@danieldaeschle danieldaeschle commented Jan 25, 2021

tbh: this error prevents me from using Go.

@spytheman
Copy link

@spytheman spytheman commented Jan 25, 2021

That's why I only complain about unused variables, which is an error that's actually annoying and at times quite workflow interrupting.

In particular, these are some examples, and only the last one is remotely useful (maybe restrict the error only to shadowed variables?). For the other ones, perhaps that shows my ineptitude, but I don't write finished code in one go. I write some unfinished code, run it, see how it behaves and go from there. Not being allowed to run it because it's unfinished is quite a hinderance.

This is so true ...
Speed of iteration is everything when prototyping.

@leonardogazio
Copy link

@leonardogazio leonardogazio commented Feb 2, 2021

Seeing as Go is a tiny bit more open minded, going from "GOPATH is the best and only way and it will always be this way absolutely totally" to soon removing it entirely, I'm taking my chances.

A lot of people find no value in this error. Furthermore, many people find it as a hinderance, a bad part of the compiler and the language.

And since the way to get rid of it is simply to _ = variable, which changes nothing in the behavior of the compiled program, this error helps solve nothing related to the running of the program. It's just an annoying reminder of "you forgot to explicitly ignore the variable before you debug and then make actual use of it later anyway like it always happens".

Therefore, according to the FAQ's reasoning: "And if it's not worth fixing, it's not worth mentioning.", Go should stop mentioning this.

And just in case, to address the potential question of "But what about people who find this useful?".

Other languages deal with this easily using linters. That way, people who find an use in this error can keep it, while not impeding both the productivity and joy of many others.

Code with unused declared vars is considered as bad practice.

@theckman
Copy link
Contributor

@theckman theckman commented Feb 2, 2021

To be quite honest, if my goal was to do rapid prototyping I would probably not be using Go as the tool for that. Maybe we should align on whether the desire to use Go in that way is reasonable / aligned with the language itself. I am not so sure.

Can you do it? Sorta, but it depends on what you're doing. I love Go and it's definitely my tool of choice, but I am not sure it's the tool I'd use if I was engineer number 1 at a startup looking to launch my product when I know rapid development / iteration will be important. It feels like a much better fit when you're more confident in the architecture / shape of the software you are building.

Realtalk, even if you make this change it's not going to be anywhere near as rapid as other languages, and I think some of that is intentional relative to the language's overall design choices. Should we be using that as an argument for change?

I never got the feeling that rapid prototyping / development is an explicit goal of Go. Heck, one of the Proverbs is "Clear is better than clever", and in my experience clever things tend to be helpful when you're trying to rapidly prototype but not necessarily build software that has a primary goal of the code being easily maintainable.

I'd rather continue to lean into system safety, and don't personally feel the trade-off here is worth it (considering the arguments presented in favor of the change). That said, if there is a compelling safety reason to consider that I would love to learn about it.

@Nathan13888
Copy link

@Nathan13888 Nathan13888 commented Feb 3, 2021

Why doesn't Go make an option for the compiler to simply give a warning instead of an error?

I feel like providing this option would make it more convincing for developers of other languages to use Go as well...

@theckman
Copy link
Contributor

@theckman theckman commented Feb 3, 2021

@Nathan13888 they address that question in the language FAQ on the topic this issue is raised on: https://golang.org/doc/faq#unused_variables_and_imports

There are two reasons for having no warnings. First, if it's worth complaining about, it's worth fixing in the code. (And if it's not worth fixing, it's not worth mentioning.) Second, having the compiler generate warnings encourages the implementation to warn about weak cases that can make compilation noisy, masking real errors that should be fixed.

@Nathan13888
Copy link

@Nathan13888 Nathan13888 commented Feb 3, 2021

I could see where they are coming from and why some people would want warnings instead.

Personally I don't mind but don't agree with giving errors for such a small thing.

Also, I don't understand why a language wants to restrict it's users in such a way. A language should provide all the tools a developer wants to see. I don't see a problem for a language to be opinionated but being restrictive is not cool.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Feb 3, 2021

I would love if this were a warning instead, like in Rust for example. I will be able to work on the code without the annoying error interrupting my workflow, but I will also be reminded to make sure my code is clean once the logic is in place.

However, since the approach of Go is "errors or nothing", I am making the case that I would much rather have nothing when it comes to unused variables.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Feb 3, 2021

@Nathan13888 One of the design goals of the Go language is to support large scale programming. This compiler error is one minor aspect of that. Experience shows us that programmers often do not clean up code after they get it working. This error forces some code cleanups to occur as the code is being developed. Making this a warning or making it optional would make the error useless, as many programmers would simply disable it always.

(For people who really can't stand it, here is a not-very-serious suggestion: it would not be hard to write a tool that adds _ = v before each return statement for each variable in the function. Then change your editor to invoke that tool on file save.)

@Nathan13888
Copy link

@Nathan13888 Nathan13888 commented Feb 4, 2021

Interesting suggestion but i think ill be fine myself. I do clean up after myself all the time so unused variables aren't really an issue for me.

@theckman
Copy link
Contributor

@theckman theckman commented Feb 4, 2021

A language should provide all the tools a developer wants to see. I don't see a problem for a language to be opinionated but being restrictive is not cool.

@Nathan13888 I appreciate your sentiment, but I am not sure that I am aligned with this. In another universe maybe I'm a carpenter, and in that world I expect my hammer to be great at hammering and my flathead screwdriver to be great at fastening things. I'm but a simple man who doesn't have time for these computer things. 😜

Assume I spend a lot of my time hammering things, and then screwing on fasteners after I do that. The naive thought would be that I could be so much more efficient at my job if my hammer just did both. But now I no longer have a hammer that's excellent at its job, I now have a frankenhammer that's mostly good at being a hammer and maybe okay at being a screwdriver. My tool can do everything I need now, but am I more effective or efficient? Probably not.

Stated another way, I wouldn't expect to build a high quality house with a Swiss Army Knife. Tools should restrict you, so that you are using them in a way that aligns with how they are designed to be used and not forced to work in a different paradigm.

I haven't heard of this word used in awhile, but it reminds me of a great German word: eierlegendewollmilchsau, which literally translates to “egg-laying wool-milk-sow”:

An all-in-one device or person which has (or claims to have) only positive attributes and which can (or attempts to) do the work of several specialized tools.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Feb 4, 2021

@ianlancetaylor

This error forces some code cleanups to occur as the code is being developed.

At the expense of impeding testing the code and running the application to see if the code works, as well as breaking or disrupting the workflow and providing a source of annoyance or frustration.

as many programmers would simply disable it always.

So what? If many programmers would disable it, perhaps it shouldn't exist.

it would not be hard to write a tool that [...]

It would be hard enough depending on the experience of someone on working with source files and would also be time consuming.

@Nathan13888
Copy link

@Nathan13888 Nathan13888 commented Feb 4, 2021

I think it doesn't hurt to add such a feature that would make everyone happy (at least not sad).

While I believe that its accurate to say that there are many people who tend to not clean up after themselves, it should at least be an option to disable such a feature that could be annoying at times. Though I personally don't think that not adding this feature would stop me from using Go, it doesn't hurt to add such a feature...

Maybe, the default is that unused variables are an error but could be temporarily ignored using a compiler flag...

I'm not sure why how come this is such an unpopular opinion as no other language I'm aware off does the same as Go in this regard. So I think its reasonable to bring up this argument or perspective.

@beoran
Copy link

@beoran beoran commented Feb 4, 2021

I have worked for 5 years on a life-critical project with millions of line of 20 years old (or older) C code, and there, we would explicitly turn on this warning and turn it into an error for new code, exactly because the old code had become a mess with unused variables left and right, or worse, variables with the wrong scope that caused values to be discarded inadvertently. So true, other languages don't do this by default to the detriment of the code quality, and the tendency there, for experienced, professional teams is to make the compiler signal it as an error.

Likewise in go like in this example:

package main

import "fmt"

func failIfEven(i int) (err error) {
	if (i % 2) == 0 {
		return fmt.Errorf("No even numbers please.")
	}
	return nil
}

func failIfSmaller(i, min int) (err error) {
	if i < min {
		return fmt.Errorf("No numbers smaller than %d please.", min)
	}
	return nil
}

func canFail(i int) (err error) {
	err = failIfEven(i)
	if err != nil {
		err := failIfSmaller(i, 10)
	}
	return err
}

func main() {
	err := canFail(6)
	fmt.Printf("canFail: %v\n", err)
}

https://play.golang.org/p/EXakALk3HXv
Spot the problem? The compiler will, thanks to this error! And this is just an example, in real code the incorrect scope problem is often even harder to notice.

@samlof
Copy link

@samlof samlof commented Feb 4, 2021

Go is easy to edit so can remove the few lines checking this actually. That way dev machine doesn't care but CICD does. Exactly what I want.

Tried doing it in my fork. Was more than expected but still simple enough.
So don't mind this proposal going either way.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Feb 4, 2021

This would mean that the system package manager cannot be used (without extra hurdles), Go must be manually built from source on every machine/OS where this modification is intended, and updates must be manually performed.

If this solution is proposed, why is it not okay to add a compiler flag that would be manually added by the dev on the development machine but that CICD will not use? It would be basically the same thing, except without all the inconveniences listed above.

@theckman
Copy link
Contributor

@theckman theckman commented Feb 5, 2021

... why is it not okay to add a compiler flag that would be manually added by the dev on the development machine but that CICD will not use? It would be basically the same thing, except without all the inconveniences listed above.

@dancojocaru2000 as a reliability engineer, a big part of the responsibility of my job is risk assessment and mitigation. Making this something that can be turned off greatly increases the risks of critical bugs being introduced into systems. How do we prevent someone from accidentally disabling that on their production build system if we do make this an option?

By making this hard to do, we discourage people from doing it. By discouraging people from doing it, we mitigate this class of risk.

Edit: It doesn't seem like your proposal addresses how it can be implemented without increasing the risk of critical flaws being introduced in systems. Maybe that would make it a more compelling change to make.

Edit2: I can't describe how I've changed my style of development over the years, but this is not a problem I encounter any longer. I definitely ran into this when I was learning Go back in the day (2013~2014), but these days it's just not a problem I hit. In talking to peers / friends after seeing this issue, it's not something they hit either. Considering that, I'm having a hard time justifying this sort of change when I consider the cost/risk it introduces, and compare it to the perceived problem that it aims to solve.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Feb 5, 2021

How do we prevent someone from accidentally disabling that on their production build system if we do make this an option?

Firstly, make it hard to do so? Perhaps if it's not their custom build system but some SAAS CI/CD platform don't even include the option to disable this?

If it will be an option, this should be an option that a developer would be able to use perhaps only for a limited amount of time before it turns itself off in order to easily develop.

How does Go prevent a program from dropping an entire table in SQL in production? It doesn't. Both because it's impossible but also because it's none of Go's business. I would argue that if someone accidentally disables that error on their production build system, firstly the option was too easy to access, but also it's none of Go's business.

It doesn't seem like your proposal addresses how it can be implemented without increasing the risk of critical flaws being introduced in systems. Maybe that would make it a more compelling change to make.

Firstly, I still fail to see how this increases the risk of critical flaws. It's not like I'm asking for importing a package that doesn't exist to be silently ignored instead of it to be an error.

The only possible counter argument that was provided apart from the syntax one is that it prevents shadowing an unused variable. Perhaps that should remain an error because that is likely unintended behavioral anyway?

but these days it's just not a problem I hit. In talking to peers / friends after seeing this issue, it's not something they hit either.

I'm certainly glad for you.

I definitely ran into this when I was learning Go back in the day (2013~2014)

However, not everybody is so fortunate as to not run into this issue. Including yourself at one point.

@beoran
Copy link

@beoran beoran commented Feb 5, 2021

How this error prevents critical errors? I gave the example of the life-critical system I programmed on before, we has some serious scope issues with variables, where a variable was declared in a lower scope with the same name as one in a higher scope. So, it was unused because it had the wrong scope, resulting in the higher scope variable not being modified correctly, and as a result the function misbehaving. Critical problems occurred, and critical as in "we were lucky no one got injured or killed". This error really can saves lives, which is why we enabled it when programming in C as gcc allows us to do this.

While it's true that the Go compiler cannot check SQL, it can check Go. I'd say, that for all these kinds of problems, where the Go compiler can see that something definitely is wrong, an error should be given. I'm actually all for having more of these kinds of errors if at all possible. Go vet is of course also great, but it's not mandatory, so it's easy to forget to run it.

You might say, my software is not life-critical. But if you write an open source library with useful functionality, it might ended up being used in life critical software, like we did also use some open source components. At first, the discipline that Go enforces may seem stifling, but give it a serious try, and I think you'll start to appreciate it once it catches a few of your own mistakes, like it did for mine.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Feb 5, 2021

So, it was unused because it had the wrong scope, resulting in the higher scope variable not being modified correctly, and as a result the function misbehaving.

I think this will fall under my proposed "no shadowed unused variabled".

where the Go compiler can see that something definitely is wrong

I 101% disagree that having an unused variable is "definitely" wrong.

But if you write an open source library with useful functionality

Then the Go compiler will not accept it when you import it because it will not allow unused variables there. Therefore, the library would be unusable until the errors are fixed.

and I think you'll start to appreciate it once it catches a few of your own mistakes, like it did for mine

I appreciate it when it comes as a warning. That way, it consistently warns me that something might be dubious, while also not being annoying, stifling, counterproductive.

When it comes as a mandatory error, the advantages are almost entirely nullified.

As for my own mistakes, I have never encountered an error caused by unused variables in all my coding experience, which is perhaps why I see close to no value in this error.

I gave the example of the life-critical system I programmed on before

This error really can saves lives, which is why we enabled it when programming in C as gcc allows us to do this.

gcc warns on unused variables. If the system is "life-critical", why does it have warnings when compiling?


And, finally, if this error is important to you, don't turn it off. Why shouldn't I be able to temporarily turn it off to facilitate a better coding experience?

This is another reason why compiling my own Go compiler with the error removed would be an inconvenience: having clean code is nice. I don't want this error to be gone forever, I just want it temporarily stopped while I'm writing the core logic and functionality and then on when I'm polishing the code and cleaning it up.

However, since the Go team is "all or nothing", no compiler flags, no warnings, this put the constraint on me to propose having the error removed entirely.

@beoran
Copy link

@beoran beoran commented Feb 5, 2021

Even if a variable it is not shadowed, if it is being used to store the results or, more likely errors returned by a function call, not using this result can likewise result in life threatening errors. Here is a fictional traffic light system example:

func switchToGreen() (bool, error) {
    err := switchOppositeDirectionToRed()
    greenErr := switchThisDirectionToGreen() 
    return greenErr
}

Here, err is not shadowed but I forgot to check err and return it, perhaps because I was under stress and forgot to finish error checking the function. Tests might not catch this, as long as the red light on the opposite side switches on correctly. But if the lamp check wire or lamp wire was damaged, we might end up with a cross road with green in one direction and no lamp on in the other direction, and then the dirivers on the no lamp side are likely to assume the green is broken, possibly leading to serious accidents...

And yes, gcc has a flag to turn warnings into errors, which we definitely used as well. That's why I think the "no warnings" approach of Go is awesome.

Also I don't think that allowing people to turn it off temporarily gives a better development experience. In stead of getting the error immediately when you want to get it to keep your code safe from the beginning, you will have to wait until your CI compile finished to get it. And then you'll have situations where CI also has it turned off ""temporarily" by your colleague who had to make an urgent fix.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Feb 5, 2021

And yes, gcc has a flag to turn warnings into errors, which we definitely used as well. That's why I think the "no warnings" approach of Go is awesome.

And that flag is not mandatory to be used, which is why I think the “no warnings” approach of Go is not awesome.

In stead [sic] of getting the error immediately when you want to get it to keep your code safe from the beginning, you will have to wait until your CI compile finished to get it.

That’s actually what will happen if I compile my own Go compiler with the error entirely removed. If it is a flag I can temporarily use while debugging, I will also have the ability to not use the flag in order to get the error and take care of things on my machine.

And then you'll have situations where CI also has it turned off ""temporarily" by your colleague who had to make an urgent fix.

  1. A good CI should not have the ability to turn the error off imo
  2. Or they can just _ = variable and done, error ignored nowadays too. So I don’t see what extra protection this is providing.
@beoran
Copy link

@beoran beoran commented Feb 5, 2021

Well, I guess we've got a widely diferrent point of view due to our different histories, so I probably have to leave it at that.

On last point about assigning to _, that is indeed the canonical way to disable this warning. Why? Because it is not a compiler flag but explicitly visible in the source code. And one of the Go design principles is that "explicit is better than implicit". So I'd say that assigning to _ is the "compiler flag" you are looking for.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Feb 5, 2021

This has two disadvantages:

  1. My code will compile on other machines too if I explicitly add _ = variable. That is unintended, since the whole idea of this is for it to be a temporary thing. Since not having unused variables is valuable, explicitly putting that in code will make the CI think it's okay, for example, which is unwanted.
  2. That is the current workflow that I'm arguing is very inconvenient: compile - error - frustration - explicitly ignore variable - recompile - works/doesn't work - hopefully remember to remove the line(s)
@beoran
Copy link

@beoran beoran commented Feb 5, 2021

In my experience. once you get used to Go then those "- error - frustration - explicitly ignore variable -" steps go away and are replaced by a "-I only make variables I really need, and then I use them immediatly-" step, followed once or twice per week by an -oh yes, the compiler was right to error on me there- step, leading to code that costs me a little bit more time to write, but is of better quality from the beginning.

But again I can only repeat our disagreement is due to our different experiences. I have written Go for 10 years so of course I'm biased in favor of this error.

I guess that one problem for Go beginners is that Go looks like C in syntax, but semantically it is more like Modula and Oberon, which like have more severe error checking than most C-like languages. So, I get that beginners might not excect that Go is as severe as a "Wirth language" are reputed to be, and that this is a hard adjustment to make, when coming from a C like language, or perhaps also from languages like JavaScript, Python or Ruby which do even less checking. I think the solution is to make better learning resources available on this point, perhaps include this error and similar in the Go tutorial.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Feb 5, 2021

I agree that once you get used to Go then almost everything in Go is perfect, including this error. However, it's hard to get used to Go when the experience using it is frustrating and quite different compared to many other langauges such as Kotlin, Swift, Rust.

And the lack of tutorials isn't really the problem. The inconvenience that the error itself brings is.

Also, writing "better quality code" from the beginning and writing better quality code 5 minutes later brings the same quality but the 2nd way also brings convenience when writing the code.

@samlof
Copy link

@samlof samlof commented Feb 5, 2021

Do you two never use the debugger to test before using or after removing broken code? At least my code isn't always without bugs.
We're lucky unreachable code isn't an error but only a lint even when it's a similar case.

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Feb 5, 2021

@samlof

Do you two

Who?

@samlof
Copy link

@samlof samlof commented Feb 5, 2021

Looks like only @beoran . Saw your "I agree that once you get used to Go then almost everything in Go is perfect, including this error." and the rest looked like you agree this error can be good.
Nvm the "you two"

@dancojocaru2000
Copy link
Author

@dancojocaru2000 dancojocaru2000 commented Feb 5, 2021

Gotcha, though you might want to read behind before jumping in since this issue (surprisingly) had some discussion.

@theckman
Copy link
Contributor

@theckman theckman commented Feb 5, 2021

And one of the Go design principles is that "explicit is better than implicit".

@beoran that's part of The Zen of Python, not Go

@beoran
Copy link

@beoran beoran commented Feb 5, 2021

@theckman Whoops, you're right. although https://talks.golang.org/2012/splash.article and other articles about Go mention the word "explicit" a lot, they don't refer to this Zen of Python in particular. It is still an important concept in Go, though.

@samlof I do use the debugger, when I don't know what my code is doing. Which is bad, and means I have to rewrite the part I am debugging, if possible, so I do understand what it is doing. The sooner a bug is found the better, and if the compiler can find bugs for me, I'm all for it, even if it costs me some convenience.

I don't consider Go the perfect language. It is hard to use well for really low level programming (graphics, OS, C interfacing), due to the garbage collector and segmented stack, but those are semantic problems, not syntax or error checking problems.

It's true in general that Go is semantically different in several ways to Rust, Kotlin or Swift, even though all four are brace block languages. I agreed, as I said before, that in the beginning, these differences are annoying. But the best way learn Go is to leave your other languages at the doorstep, and accept those differences in stead of struggling against them. Then everything falls into place that much sooner.

@ingvarm-gr
Copy link

@ingvarm-gr ingvarm-gr commented Feb 8, 2021

@samlof

Personally, I tend towards "extensive debug logging", which naturally causes any variable I want to see what happens to being used (because it'll be referenced from the debug logging statement). It is also frequently the case (for me) that my workstation is not a suitable environment for the binary to run (the code I write tends to run in kubernetes clusters and frequently needs to work with different RBAC restrictions than my account has), so attaching a debugger to the running binary is actually seldom an option for me.

I think I last intentionally attached a debugger to code I've written well over a decade ago.

@tux3
Copy link

@tux3 tux3 commented Mar 10, 2021

I think it's completely fair for Golang to have a different philosophy and to do its own thing. Go is different from other languages in many ways, and there's value in that. But I also completely empathize with people who are annoyed by unused variable warnings.

Personally, this error really breaks my flow, but I understand that there's a different point of view, that Go is opinionated and forces people to fix things, and it's a reasonable one.
So, a note for others like me: as long as you fix up and clean your code before pushing, it's totally okay to have your own opinion and your own preferences, too. And to run your own code, if that's what you like.
Go is open source, and upstream has no obligation to take your patches. But, if the compiler causes you pain, there is always the hacker option: Fork It On Github! 😄

The code in the compiler responsible for this behavior actually doesn't change very often, if it bothers you it's totally possible to apply a small diff and rebase it when a new version comes out. I don't recommend you go and blindly apply this locally, but to put my money code where my mouth is, here's a naive 75 line diff for Go 1.16 that turns "unused" errors into warnings.

Now, a disclaimer: The patch above is a quick, dirty hack and not intended for wide/serious/production use, and especially not intended to be merged upstream! If your laptop catches fire as a result of running this patch, or if your code is miscompiled, I will merily deny all responsibility! When I say that it's a PoC, I mean it! Don't expect the test suite to pass =)
And if you decide to turn errors into warnings on your machine, please be considerate and make sure you always fix those. No one else should bear the cost of your different workflow.

Go has its own philosophy, maybe you have yours too. And maybe that's okay 🙂
Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Proposals
Incoming
Linked pull requests

Successfully merging a pull request may close this issue.

None yet