-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: write a test for #30167 #30664
Comments
@mvdan May I carry on the task to work on this issue please? |
It's not assigned, so you're welcome to try. Though I think @dr2chase has specific plans on how to fix it, and they're non-trivial. |
Ah, plans are changing slightly, and might become simpler. I experimented with gdb-MI a little bit, and it appears that it might still be flaky (I am not 100% sure of this), plus in practice, people use Delve, not gdb, because gdb doesn't do a good job with goroutines. And Delve isn't flaky. I have to be sure that Delve is supported on our test boxes, and then we switch to that as the default. The fix I imagine is to enhance debug_test.go to do one of the following:
And then use that to write the test. |
Change https://golang.org/cl/168477 mentions this issue: |
It needs more than just a test; there's something buggy going on, but I won't get to it today.
It loops like we should maybe notice that we are creating a one-instruction infinite loop, and throw a nop in there? I'm not sure, it depends on debugger heuristics, but it is definitely tagged as a statement. That would make the infinite loop take longer to finish.... |
https://go-review.googlesource.com/c/go/+/163019 was sent and submitted without a test, because writing one wasn't possible with the current test suite. David has an idea to refactor the tests to support this kind of test case (see the CL comments); this issue is a reminder to write that test in the future.
/cc @pwaller @dr2chase
The text was updated successfully, but these errors were encountered: