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/errtrace: Fix blank import handling #92

Merged
merged 1 commit into from
Feb 18, 2024
Merged

cmd/errtrace: Fix blank import handling #92

merged 1 commit into from
Feb 18, 2024

Conversation

prashantv
Copy link
Contributor

With toolexec mode, a blank import of errtrace is more likely to opt-in to rewriting. However, this blank import can't be used to call errtrace functions, so we need to track both:

  • Whether errtrace is imported (used to opt-in toolexec rewriting)
  • Whether errtrace needs to be imported for any Wrap calls.

Since this change modifies the rewriting logic, we bump the errtrace toolexec version manually.

Copy link
Contributor

@abhinav abhinav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

I was wondering in the other PR why we weren't putting import _ in the same file, and then I saw this PR.

Base automatically changed from prashant/toolexec-1 to main February 18, 2024 06:33
With toolexec mode, a blank import of errtrace is more likely
to opt-in to rewriting. However, this blank import can't be used
to call errtrace functions, so we need to track both:

 * Whether errtrace is imported (used to opt-in toolexec rewriting)
 * Whether errtrace needs to be imported for any Wrap calls.

Since this change modifies the rewriting logic, we bump the errtrace
toolexec version manually.
@prashantv
Copy link
Contributor Author

Nice.

I was wondering in the other PR why we weren't putting import _ in the same file, and then I saw this PR.

Yep, though I'm leaving p3's import of errtrace in a separate file to test the package-level opt-in

@prashantv prashantv merged commit 02aad23 into main Feb 18, 2024
12 checks passed
@prashantv prashantv deleted the prashant/import branch February 18, 2024 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants