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.

Sign up for GitHub

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

x/tools/go/analysis/passes/fieldalignment: "struct with X pointer bytes could be Y" linting errors for (what appears to be) aligned structs #45541

Closed
atc0005 opened this issue Apr 13, 2021 · 6 comments
Closed

x/tools/go/analysis/passes/fieldalignment: "struct with X pointer bytes could be Y" linting errors for (what appears to be) aligned structs #45541

atc0005 opened this issue Apr 13, 2021 · 6 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@atc0005
Copy link

atc0005 commented Apr 13, 2021

What version of Go are you using (go version)?

$ go version
go version go1.16.3 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\atc0005\AppData\Local\go-build
set GOENV=C:\Users\atc0005\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\atc0005\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\atc0005\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.16.3
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=T:\github\help-requests\go\struct-alignment\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\atc0005\AppData\Local\Temp\go-build4294469141=/tmp/go-build -gno-record-gcc-switches

What did you do?

Example structs:

	type emailConfigAscending struct {
		timeout                time.Duration      // 8 bytes
		notificationRateLimit  time.Duration      // 8 bytes
		template               *template.Template // 8 bytes
		serverPort             int                // 8 bytes
		notificationRetries    int                // 8 bytes
		notificationRetryDelay int                // 8 bytes
		server                 string             // 16 bytes
		senderAddress          string             // 16 bytes
		clientIdentity         string             // 16 bytes
		recipientAddresses     []string           // 24 bytes
	}

	type emailConfigDescending struct {
		recipientAddresses     []string           // 24 bytes
		server                 string             // 16 bytes
		senderAddress          string             // 16 bytes
		clientIdentity         string             // 16 bytes
		timeout                time.Duration      // 8 bytes
		notificationRateLimit  time.Duration      // 8 bytes
		template               *template.Template // 8 bytes
		serverPort             int                // 8 bytes
		notificationRetries    int                // 8 bytes
		notificationRetryDelay int                // 8 bytes
	}

I first encountered the linting errors via CI Docker container (govet fieldalignment golangci-lint linter), then reproduced by enabling fieldalignment vscode Go plugin analyzer.

What did you expect to see?

No linting errors.

What did you see instead?

These linting errors for two variations of the struct:

  • fieldalignment: struct with 104 pointer bytes could be 64 (govet)
  • fieldalignment: struct with 96 pointer bytes could be 64 (govet)
@seankhliao seankhliao changed the title fieldalignment: "struct with X pointer bytes could be Y" linting errors for (what appears to be) aligned structs x/tools/go/analysis/passes/fieldalignment: "struct with X pointer bytes could be Y" linting errors for (what appears to be) aligned structs Apr 13, 2021
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Apr 13, 2021
@gopherbot gopherbot added this to the Unreleased milestone Apr 13, 2021
@seankhliao
Copy link
Member

cc @leitzler

@seankhliao seankhliao added Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 13, 2021
@leitzler
Copy link
Contributor

The linting errors you see is about pointer bytes, not about size. As described in #44877 (comment) that is how many bytes of the object that the GC has to potentially scan for pointers.

We should probably clarify the documentation to reflect that there are two different diagnostics reported, struct of size x could be y and struct with x pointer bytes could be y, and how they differ.

I suggest closing this issue as a duplicate of #44877.

@stamblerre
Copy link
Contributor

@leitzler: Do you want to send a CL to clarify the documentation before we close this issue (since #44877 is closed)?

@atc0005 atc0005 mentioned this issue Apr 14, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/314469 mentions this issue: go/analysis/passes/fieldalignment: clarify reported diagnostics in docs

@leitzler
Copy link
Contributor

Apologies for the delay, sent a CL that hopefully clarifies the docs!

@atc0005
Copy link
Author

atc0005 commented Apr 27, 2021

@leitzler is there support for enabling only one of the diagnostics for that analyzer? For example, enabling just the struct size check and leaving the "pointer bytes" used disabled?

@golang golang locked and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@leitzler @stamblerre @gopherbot @seankhliao @atc0005