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

cmd/compile: spurious unused errors for used imports in expected function calls e.g defers and go #23586

Closed
dotaheor opened this issue Jan 28, 2018 · 7 comments

Comments

Projects
None yet
5 participants
@dotaheor
Copy link

commented Jan 28, 2018

What version of Go are you using (go version)?

go version go1.9.2 linux/amd6

Does this issue reproduce with the latest release?

yes

What did you do?

package main

import "fmt" // error: imported and not used: "fmt"

func main() {
	defer func() {
		fmt.Println()
	}
	go func() {
		fmt.Println()
	}
}

What did you expect to see?

./a.go:8:3: expression in defer must be function call
./a.go:11:3: expression in go must be function call

What did you see instead?

./a.go:3:8: imported and not used: "fmt"
./a.go:8:3: expression in defer must be function call
./a.go:11:3: expression in go must be function call
@agnivade

This comment has been minimized.

Copy link
Member

commented Jan 28, 2018

It is not unnecessary. It is by design - https://golang.org/doc/faq#unused_variables_and_imports.

The presence of an unused variable may indicate a bug, while unused imports just slow down compilation, an effect that can become substantial as a program accumulates code and programmers over time. For these reasons, Go refuses to compile programs with unused variables or imports, trading short-term convenience for long-term build speed and program clarity.

@mvdan

This comment has been minimized.

Copy link
Member

commented Jan 28, 2018

I think OP's point is that fmt is used in the program, even if it's in a malformed go/defer expression.

Smaller reproducer:

package main

import "fmt"

func main() {
        defer func() {
                _ = fmt.Print
        }
}

Perhaps this is by design. I don't know how far are invalid expressions typechecked. /cc @griesemer @mdempsky

@griesemer griesemer self-assigned this Jan 28, 2018

@griesemer griesemer added this to the Go1.11 milestone Jan 28, 2018

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2018

I agree that the compiler could do better in this case; there's really just one error here (the deferred function must be invoked). go/types avoids the unused fmt error. This should probably be fixed.

The compiler is also not consistent. For instance, in this case

package main

func main() {
	var i int
        defer func() {}
}

we have the opposite situation; i is indeed not used, but there's no error. (Interestingly go/types also doesn't complain in this case; I will file a separate bug for that).

@griesemer griesemer added the NeedsFix label Jan 28, 2018

@odeke-em odeke-em changed the title cmd/compile: unnecessary compile error message cmd/compile: spurious unused errors for used imports in expected function calls e.g defers and go Jan 29, 2018

@dotaheor

This comment has been minimized.

Copy link
Author

commented Feb 1, 2018

@griesemer I think the case in your comment is acceptable.
After all, "deferred function not called" is a more serious error than "a variable not used".

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2018

The root cause for the current behavior is that the check that the defer expression is a function call happens at parse time; and if there are parse errors the compiler stops (and doesn't bother type-checking the code).

In this case it may make sense to move that test to later (out of the parser) for a couple of reasons: 1) it is quite common to make the mistake of forgetting to call the deferred function; and 2) the syntax actually doesn't state that the expression must be a function call, that restriction in made in prose. As a general (but not hard and fast) rule for the parser, syntactic restrictions that are in prose are handled outside the parser.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 15, 2018

Change https://golang.org/cl/94155 mentions this issue: cmd/compile/internal/syntax: more tolerant handling of missing function invocation in go/defer

@gopherbot

This comment has been minimized.

Copy link

commented Feb 15, 2018

Change https://golang.org/cl/94156 mentions this issue: cmd/compile/internal/syntax: more tolerant handling of missing function invocation in go/defer

@gopherbot gopherbot closed this in 1a22738 Feb 15, 2018

@golang golang locked and limited conversation to collaborators Feb 15, 2019

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