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: eliminate empty switch cases #24650

Open
josharian opened this Issue Apr 2, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@josharian
Contributor

josharian commented Apr 2, 2018

package p

var y int

func f(x int) {
	switch x {
	case 1:
	case 2:
		y = 1
	}
}

This compiles to:

"".f STEXT nosplit size=31 args=0x8 locals=0x0
	0x0000 00000 (x.go:5)	TEXT	"".f(SB), NOSPLIT, $0-8
	0x0000 00000 (x.go:5)	FUNCDATA	$0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x0000 00000 (x.go:5)	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x0000 00000 (x.go:5)	MOVQ	"".x+8(SP), AX
	0x0005 00005 (x.go:7)	CMPQ	AX, $1
	0x0009 00009 (x.go:7)	JNE	12
	0x000b 00011 (<unknown line number>)	RET
	0x000c 00012 (x.go:8)	CMPQ	AX, $2
	0x0010 00016 (x.go:8)	JNE	11
	0x0012 00018 (x.go:9)	MOVQ	$1, "".y(SB)
	0x001d 00029 (x.go:6)	JMP	11

Note that we're pointlessly testing whether x == 1 at 0x0005. We should be able to eliminate that test. Probably the easiest place is when lowering the switch statement; if there is no default case, then any cases with empty bodies can be removed. (Taking care to evaluate the case expressions if necessary, of course, and taking care with pointless fallthroughs from the previous case.) Another possibility, probably more powerful, is to detect the unnecessary branch somehow in SSA form.

This kind of code arises naturally when wanting to make it clear to the reader that a particular case has been considered by the code author. It also shows up in generated code.

@josharian josharian added this to the Unplanned milestone Apr 2, 2018

@bradfitz bradfitz added the NeedsFix label Apr 3, 2018

@ghost

This comment has been minimized.

ghost commented Apr 26, 2018

Consider the situation where comparisons in later cases take advantage of guarantees from an earlier, empty case, e.g. this example from net/http/transport.go.

switch {
case cm.proxyURL == nil:
	// Do nothing. Not using a proxy.
case cm.proxyURL.Scheme == "socks5":

If we eliminate the empty case and cm.proxyURL is nil, the next case will trigger a null pointer dereference and a panic.

This is not exclusive to true/false switches. It could happen in switches on a constant:

switch nil {
case x:
case y:
	x.f()
}

or on a variable (swap x and nil)

Not even analysis for comparisons to nil would help, e.g.

s := make([]int, 1)
switch x {
case 1:
case y:
	_ = s[x]
@go101

This comment has been minimized.

go101 commented Apr 26, 2018

@IronGopher op's example is some special, in which the right operands following case are all different constants.

@go101

This comment has been minimized.

go101 commented Apr 27, 2018

But the assumed optimization may make programs run slower sometimes,
For example, if the case 1 route is the most frequent branch get executed.

func f(x int) {
	switch x {
	case 1:
	case 2:
		y = 1
	case 3:
		y = 2
	case 4:
		y = 3
	}
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment