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.

Sign up for GitHub

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

Jump to bottom

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

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

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

xiyichan opened this issue Sep 17, 2020 · 5 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Backlog

Comments

@xiyichan
Copy link
Contributor

xiyichan commented Sep 17, 2020 •
edited
Loading

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
Contributor

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

@mdempsky I passed all. bash.

@mdempsky
Copy link
Contributor

mdempsky commented Sep 21, 2020 •
edited
Loading

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

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

@toothrot toothrot added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Sep 21, 2020
@toothrot toothrot added this to the Backlog milestone Sep 21, 2020
@toothrot toothrot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 21, 2020
@gopherbot
Copy link
Contributor

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

@xiyichan xiyichan closed this as completed Feb 8, 2021
@golang golang locked and limited conversation to collaborators Feb 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@toothrot @mdempsky @gopherbot @xiyichan