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

Jump to bottom

cmd/go: revert special-case allowing "sync" to import "runtime/internal/atomic" #42196

Closed
hyangah opened this issue Oct 25, 2020 · 6 comments
Closed

cmd/go: revert special-case allowing "sync" to import "runtime/internal/atomic" #42196

hyangah opened this issue Oct 25, 2020 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.16

Comments

@hyangah
Copy link
Contributor

hyangah commented Oct 25, 2020 

[Trace - 09:15:51.459 AM] Sending request 'textDocument/codeAction - (10)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/go_tip/go/src/sync/pool.go"},"range":{"start":{"line":9,"character":15},"end":{"line":9,"character":40}},"context":{"diagnostics":[{"range":{"start":{"line":9,"character":15},"end":{"line":9,"character":40}},"message":"could not import runtime/internal/atomic (invalid use of internal package runtime/internal/atomic)","severity":1,"source":"compiler"}],"only":["quickfix"]}}

  • How to reproduce:

    • Sync at tip
    • run ./make.bash
    • Open the src folder

I am using vscode with the following setting:

 {
  "go.goroot": "/Users/hakim/go_tip/go",
  "go.useLanguageServer": true
}

Not sure if this is a transient error of go compiler (but make.bash didn't complain) or if this is an error caused by editor misconfiguration.

  • Additional info
~/go_tip/go/src$ go version
go version devel +94887410d4 Sun Oct 25 07:51:58 2020 +0000 darwin/amd64

Used gopls v0.5.1, but could reproduce it with v0.5.2-pre.1.

[Info  - 9:15:36 AM] 2020/10/25 09:15:35 go env for /Users/hakim/go_tip/go/src
(root /Users/hakim/go_tip/go/src)
(go version go version devel +94887410d4 Sun Oct 25 07:51:58 2020 +0000 darwin/amd64
)
(valid build configuration = true)
(build flags: [])
GOPATH=/Users/hakim/go
GOPROXY=https://proxy.golang.org,direct
GOCACHE=/Users/hakim/Library/Caches/go-build
GOFLAGS=
GOSUMDB=sum.golang.org
GO111MODULE=auto
GONOPROXY=
GOPRIVATE=
GOINSECURE=
GONOSUMDB=
GOROOT=/Users/hakim/go_tip/go
GOMOD=/Users/hakim/go_tip/go/src/go.mod
GOMODCACHE=/Users/hakim/go/pkg/mod
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Oct 25, 2020
@gopherbot gopherbot added this to the Unreleased milestone Oct 25, 2020
@heschi
Copy link
Contributor

heschi commented Oct 26, 2020

@bcmills is reverting the cmd/go change that makes this work for make.bash.

@gopherbot
Copy link

Change https://golang.org/cl/265122 mentions this issue: Revert "cmd/compiler,cmd/go,sync: add internal {LoadAcq,StoreRel}64 on ppc64"

@bcmills bcmills modified the milestones: Unreleased, Go1.16 Oct 26, 2020
@bcmills
Copy link
Contributor

bcmills commented Oct 26, 2020

Marking as release-blocker for Go 1.16 because this regression was introduced by a change in cmd/go in Go 1.16.

@bcmills bcmills changed the title x/tools/gopls: reports an 'invalid use of internal package' error in sync/pool.go cmd/go: revert special-case allowing "sync" to import "runtime/internal/atomic" Oct 26, 2020
@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. and removed Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Oct 26, 2020
@laboger
Copy link
Contributor

laboger commented Oct 26, 2020

@bcmills What needs to be done for this CL? This was done based on a suggestion from Austin, but obviously not in the preferred way.

@bcmills
Copy link
Contributor

bcmills commented Oct 26, 2020

We definitely need to revert the change in src/cmd/go/internal/load/pkg.go, since that's adding the new special case that gopls is stumbling over.

It may be possible to rework the code to use go:linkname instead of directly importing the internal package. (I believe @aclements is looking into that now.) Otherwise, one possible approach might be to move runtime/internal/atomic to internal/atomic so that it can be imported in keeping with the existing internal rules.

@gopherbot
Copy link

Change https://golang.org/cl/265124 mentions this issue: cmd/go,cmd/compile,sync: remove special import case in cmd/go

@golang golang locked and limited conversation to collaborators Oct 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.16
Development

No branches or pull requests
6 participants
@aclements @hyangah @bcmills @gopherbot @laboger @heschi