Skip to content

go/doc: avoid panic on references to functions with no body #43011

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

Closed
wants to merge 3 commits into from

Conversation

qbradq
Copy link
Contributor

@qbradq qbradq commented Dec 4, 2020

This change guards a call to ast.Inspect with a nil check on the first
argument. This avoids a panic when inspecting a reference to a function
with a nil body. This can only happen when a function body is defined outside Go.

Fixes #42706

@google-cla google-cla bot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Dec 4, 2020
@qbradq
Copy link
Contributor Author

qbradq commented Dec 4, 2020

I'm not sure the test case is in the most logical place. The whole file test section allowed easy recreation of the issue's example.

@gopherbot
Copy link
Contributor

This PR (HEAD: ad078e6) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/275516 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@qbradq qbradq changed the title go/doc: avoid panic on references functions with no body go/doc: avoid panic on references to functions with no body Dec 4, 2020
@gopherbot
Copy link
Contributor

Message from Daniel Martí:

Patch Set 2: Run-TryBot+1 Trust+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 2:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=3a09e7e5


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 2: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Norman Lancaster:

Patch Set 3:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Daniel Martí:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Norman Lancaster:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Daniel Martí:

Patch Set 3: Code-Review+2 Trust+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Emmanuel Odeke:

Patch Set 3: Trust+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

@odeke-em
Copy link
Member

@qbradq please rebase your PR with the latest from the master branch so that we can run tests, and then merge it. Thank you!

qbradq added 3 commits March 30, 2021 18:28
This change guards a call to ast.Inspect with a nil check
on the first argument. This avoids a panic when inspecting a
reference to a function with an external body.

Fixes golang#42706
@gopherbot
Copy link
Contributor

This PR (HEAD: 08072b9) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/275516 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Hyang-Ah Hana Kim:

Patch Set 4: Patch Set 3 was rebased


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Daniel Martí:

Patch Set 5: Run-TryBot+1 Code-Review+2 Trust+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 5:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=49553924


Please don’t reply on this GitHub thread. Visit golang.org/cl/275516.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Mar 30, 2021
This change guards a call to ast.Inspect with a nil check on the first
argument. This avoids a panic when inspecting a reference to a function
with a nil body. This can only happen when a function body is defined outside Go.

Fixes #42706

Change-Id: I91bc607b24b6224920c24cfd07e76ce7737a98d4
GitHub-Last-Rev: 08072b9
GitHub-Pull-Request: #43011
Reviewed-on: https://go-review.googlesource.com/c/go/+/275516
Reviewed-by: Daniel Martí <[email protected]>
Trust: Daniel Martí <[email protected]>
Trust: Emmanuel Odeke <[email protected]>
Run-TryBot: Daniel Martí <[email protected]>
TryBot-Result: Go Bot <[email protected]>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/275516 has been merged.

@gopherbot gopherbot closed this Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

go/doc: Examples panics on a file containing a function without a body
3 participants