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

Remove last remaining !go1.22 build tag #9197

Merged
merged 1 commit into from
Jul 21, 2023
Merged

Conversation

copybara-service[bot]
Copy link

@copybara-service copybara-service bot commented Jul 20, 2023

Remove last remaining !go1.22 build tag

The last remaining !go1.22 build is protecting the definition of
pkg/sync.maptype, which is a copy of runtime.maptype. We need to ensure these
definitions match so we can safely access the hasher field.

At its core, this CL achieves this check by ensuring that
unsafe.Offsetof(maptype{}.Hasher) matches the offset in the runtime version of
the type.

Several things happen along the way to achieve this:

  • As of May 2023, runtime.maptype is actually a type alias for
    internal/abi.MapType. checkoffset was failing to record the offsets because it
    skipped type aliases for no good reason. Simply removing the type alias check
    is sufficient to make type aliases work. (This part of the CL is technically
    unnecessary because this CL ultimately references internal/abi.MapType
    directly in anticipation of removal of the type alias. But there is no reason
    not to allow type aliases).

  • The checkconst / checkoffset regexp unintentionally does not allow / in
    package paths, even though the rest of the package supports /. Fix this.

  • checkconst was comparing the literal AST expression string against the
    runtime value (i.e., "unsafe.Offsetof(maptype{}.Hasher)" vs "72", which fails
    comparison. Switch to getting the resolved constant value from the type
    checker.

  • nogo/check.importer only loads package facts on direct import (stored in
    importer.cache). If a package is not directly imported ImportPackageFact will
    not find the facts. Typically packages need to ensure they directly depend on
    packages they want facts from (e.g., pkg/sync has a dummy import of runtime in
    runtime.go). This doesn't work for internal/abi because we cannot directly
    import an internal package. Work around this as a hack by unconditionally
    "importing" internal/abi when analyzing any package.

With regard to the last point, not that the nogo/defs.bzl nogo integration only
provides facts from the direct dependencies and the entire stdlib (since the
stdlib is analyzed as one bundle). So this trick only works for a stdlib
package. A bazel package indirect dependency would be missing facts altogether.

@copybara-service copybara-service bot added the exported Issue was exported automatically label Jul 20, 2023
@avagin avagin mentioned this pull request Jul 21, 2023
@copybara-service copybara-service bot force-pushed the test/cl549686660 branch 3 times, most recently from 874a699 to 451b29f Compare July 21, 2023 17:51
The last remaining !go1.22 build is protecting the definition of
pkg/sync.maptype, which is a copy of runtime.maptype. We need to ensure these
definitions match so we can safely access the hasher field.

At its core, this CL achieves this check by ensuring that
unsafe.Offsetof(maptype{}.Hasher) matches the offset in the runtime version of
the type.

Several things happen along the way to achieve this:

* As of May 2023, runtime.maptype is actually a type alias for
internal/abi.MapType. checkoffset was failing to record the offsets because it
skipped type aliases for no good reason. Simply removing the type alias check
is sufficient to make type aliases work. (This part of the CL is technically
unnecessary because this CL ultimately references internal/abi.MapType
directly in anticipation of removal of the type alias. But there is no reason
not to allow type aliases).

* The checkconst / checkoffset regexp unintentionally does not allow / in
package paths, even though the rest of the package supports /. Fix this.

* checkconst was comparing the literal AST expression string against the
runtime value (i.e., "unsafe.Offsetof(maptype{}.Hasher)" vs "72", which fails
comparison. Switch to getting the resolved constant value from the type
checker.

* nogo/check.importer only loads package facts on direct import (stored in
importer.cache). If a package is not directly imported ImportPackageFact will
not find the facts. Typically packages need to ensure they directly depend on
packages they want facts from (e.g., pkg/sync has a dummy import of runtime in
runtime.go). This doesn't work for internal/abi because we cannot directly
import an internal package. Work around this as a hack by unconditionally
"importing" internal/abi when analyzing any package.

With regard to the last point, not that the nogo/defs.bzl nogo integration only
provides facts from the direct dependencies and the entire stdlib (since the
stdlib is analyzed as one bundle). So this trick only works for a stdlib
package. A bazel package indirect dependency would be missing facts altogether.

PiperOrigin-RevId: 549999084
@copybara-service copybara-service bot merged commit f3e4a1f into master Jul 21, 2023
1 check passed
@copybara-service copybara-service bot deleted the test/cl549686660 branch July 21, 2023 18:22
dangra pushed a commit to superfly/flyctl that referenced this pull request Jan 10, 2024
Support Go 1.22

ran:
go get golang.zx2c4.com/wireguard@latest
to get also the updated gvisor.dev/gvisor with Go 1.22 support from google/gvisor#9197

Fixes #3145
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exported Issue was exported automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant