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: About simplifying the if of typecheck1 #41446

Open
xiyichan opened this issue Sep 17, 2020 · 5 comments
Open

cmd/compile: About simplifying the if of typecheck1 #41446

xiyichan opened this issue Sep 17, 2020 · 5 comments
Labels
Milestone

Comments

@xiyichan
Copy link
Contributor

@xiyichan xiyichan commented Sep 17, 2020

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

$ go version
master

Does this issue reproduce with the latest release?

yes

What did you do?

I find a todo tag in line 2106 of the file cmd/compile/internal/gc/typecheck.go.Is this simplifying the 'if'?

if (top&ctxStmt != 0) && top&(ctxCallee|ctxExpr|ctxType) == 0 && ok&ctxStmt == 0 {
	if !n.Diag() {
		yyerror("%v evaluated but not used", n)
		n.SetDiag(true)
	}
	n.Type = nil
	return n
}

I found that this code will enter the 'if' when top is equal to 0001. Can it be changed like this?

if top&ctxStmt == ctxStmt && ok&ctxStmt == 0 {
	if !n.Diag() {
		yyerror("%v evaluated but not used", n)
		n.SetDiag(true)
	}
	n.Type = nil
	return n
}

Then another 'if' I have no idea to simplify.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Sep 20, 2020

You can run all.bash in $GOROOT/src to see if Go still builds and the regression tests still pass. I suspect tests will fail though. (I trust that rsc included that middle comparison for a reason.)

@xiyichan
Copy link
Contributor Author

@xiyichan xiyichan commented Sep 21, 2020

@mdempsky I passed all. bash.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Sep 21, 2020

Interesting. If you want to upload a CL for that, I'll review it.

It would be nice if we can change all 4 of those if statements into something like:

if top & ok == 0 {
    switch {
    case ...:
        yyerror(...)
    ...
    default:
        Fatalf(...)
    }
    n.Type = nil
    return n
}

However, I don't know how to fill in the ...s without looking into this more myself.

@xiyichan
Copy link
Contributor Author

@xiyichan xiyichan commented Sep 21, 2020

I try to uplaod a CL, but maybe not all can be simplified.

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 22, 2020

Change https://golang.org/cl/256477 mentions this issue: cmd/compile: simplify typcheck1

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.