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

go/parser: back-out or document the SkipObjectResolution mode pending #46485 #46298

Open
findleyr opened this issue May 20, 2021 · 9 comments
Open

Comments

@findleyr
Copy link
Contributor

@findleyr findleyr commented May 20, 2021

+pkg go/parser, const SkipObjectResolution = 64 +pkg go/parser, const SkipObjectResolution Mode

The SkipObjectResolution mode was added to allow skipping the object resolution phase of parsing.

CC @griesemer

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 28, 2021

This one also doesn't seem to have gone through the proposal process. The issue discussing it is #45104.

@findleyr
Copy link
Contributor Author

@findleyr findleyr commented May 28, 2021

Indeed. @griesemer and I did discuss making a proposal at the time, and thought it wasn't warranted. If you or anyone feels differently, I don't mind unexporting the API and waiting for 1.18.

@griesemer
Copy link
Contributor

@griesemer griesemer commented May 28, 2021

Agreed. This is a just an additional mode, and nothing is expected to break if the mode is ignored.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 28, 2021

Sure, I don't expect this to be a breaking change or anything, but normally new API goes through the proposal process.

If this is needed for generics then I think it is covered by that proposal, but that's not obvious to me in this case.

@findleyr
Copy link
Contributor Author

@findleyr findleyr commented May 28, 2021

This is not directly needed for generics. As a side-effect of the refactoring I was doing anyway, this optimization, which is desirable for some users, became trivial.

We can unexport this mode value and make a proposal for 1.18. Users can experiment with it in 1.17 by copying the mode value into their project.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 28, 2021

It's OK to make it a proposal for 1.17 if you like. I don't think it would take long to approve. Thanks.

@findleyr
Copy link
Contributor Author

@findleyr findleyr commented Jun 1, 2021

Thanks, I've filed #46485 for the proposal. If the discussion there is one-sided we can perhaps get it in for 1.17, but I don't want to rush it.

For now I'll un-export the new mode to close this release blocker.

@findleyr findleyr changed the title doc/go1.17: document go/parser changes for Go 1.17 go/parser: back-out SkipObjectResolution pending #46485 Jun 1, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 1, 2021

Change https://golang.org/cl/323909 mentions this issue: go/parser: un-export the SkipObjectResolution mode pending discussion

@findleyr findleyr changed the title go/parser: back-out SkipObjectResolution pending #46485 go/parser: back-out or document the SkipObjectResolution mode pending #46485 Jun 2, 2021
@findleyr
Copy link
Contributor Author

@findleyr findleyr commented Jun 2, 2021

I discussed this issue with @golang/release today, and consensus was that it can be marked okay-after-beta-1. Reverting the mode is trivial and safe if #46485 is not approved. Leaving the mode in the release for now allows users to more easily experiment with it.

Depending on the outcome of the proposal, I will either document or revert this mode in ~2 weeks.

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
5 participants