gopls/internal/settings: remove fieldalignment analyzer
Since it doesn't find likely mistakes, it is a poor fit for the
gopls analyzer suite: even when off-by-default, its diagnostics
can be confusing.
Instead, its docs now advise users who come across it to run
it using a standalone singlechecker as desired.
+ release note
Also, we issue a deprecated warning if the user's configuration
enables the deleted analyzer, with a reference to the 0.17
release so that users can find the release note.
Fixes golang/go#67762
Change-Id: I7e2eafc3216df84eb62de132ac2f04e0bf444f92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/590375
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/go/analysis/passes/fieldalignment/fieldalignment.go b/go/analysis/passes/fieldalignment/fieldalignment.go
index 012e2ec..8af717b 100644
--- a/go/analysis/passes/fieldalignment/fieldalignment.go
+++ b/go/analysis/passes/fieldalignment/fieldalignment.go
@@ -46,6 +46,15 @@
In rare cases it may cause two variables each updated by its own goroutine
to occupy the same CPU cache line, inducing a form of memory contention
known as "false sharing" that slows down both goroutines.
+
+Unlike most analyzers, which report likely mistakes, the diagnostics
+produced by fieldanalyzer very rarely indicate a significant problem,
+so the analyzer is not included in typical suites such as vet or
+gopls. Use this standalone command to run it on your code:
+
+ $ go install golang.org/x/tools/go/analysis/passes/fieldalignment/cmd/fieldalignment@latest
+ $ go fieldalignment [packages]
+
`
var Analyzer = &analysis.Analyzer{
diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index b590120..d3f7ba8 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -257,40 +257,6 @@
Package documentation: [errorsas](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/errorsas)
-<a id='fieldalignment'></a>
-## `fieldalignment`: find structs that would use less memory if their fields were sorted
-
-
-This analyzer find structs that can be rearranged to use less memory, and provides
-a suggested edit with the most compact order.
-
-Note that there are two different diagnostics reported. One checks struct size,
-and the other reports "pointer bytes" used. Pointer bytes is how many bytes of the
-object that the garbage collector has to potentially scan for pointers, for example:
-
- struct { uint32; string }
-
-have 16 pointer bytes because the garbage collector has to scan up through the string's
-inner pointer.
-
- struct { string; *uint32 }
-
-has 24 pointer bytes because it has to scan further through the *uint32.
-
- struct { string; uint32 }
-
-has 8 because it can stop immediately after the string pointer.
-
-Be aware that the most compact order is not always the most efficient.
-In rare cases it may cause two variables each updated by its own goroutine
-to occupy the same CPU cache line, inducing a form of memory contention
-known as "false sharing" that slows down both goroutines.
-
-
-Default: off. Enable by setting `"analyses": {"fieldalignment": true}`.
-
-Package documentation: [fieldalignment](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/fieldalignment)
-
<a id='fillreturns'></a>
## `fillreturns`: suggest fixes for errors due to an incorrect number of return values
diff --git a/gopls/doc/release/v0.17.0.md b/gopls/doc/release/v0.17.0.md
new file mode 100644
index 0000000..3fbfa73
--- /dev/null
+++ b/gopls/doc/release/v0.17.0.md
@@ -0,0 +1,7 @@
+
+
+# Configuration Changes
+
+The `fieldalignment` analyzer, previously disabled by default, has
+been removed: it is redundant with the hover size/offset information
+displayed by v0.16.0 and its diagnostics were confusing.
diff --git a/gopls/internal/doc/api.json b/gopls/internal/doc/api.json
index 2deec3a..2745a43 100644
--- a/gopls/internal/doc/api.json
+++ b/gopls/internal/doc/api.json
@@ -448,11 +448,6 @@
"Default": "true"
},
{
- "Name": "\"fieldalignment\"",
- "Doc": "find structs that would use less memory if their fields were sorted\n\nThis analyzer find structs that can be rearranged to use less memory, and provides\na suggested edit with the most compact order.\n\nNote that there are two different diagnostics reported. One checks struct size,\nand the other reports \"pointer bytes\" used. Pointer bytes is how many bytes of the\nobject that the garbage collector has to potentially scan for pointers, for example:\n\n\tstruct { uint32; string }\n\nhave 16 pointer bytes because the garbage collector has to scan up through the string's\ninner pointer.\n\n\tstruct { string; *uint32 }\n\nhas 24 pointer bytes because it has to scan further through the *uint32.\n\n\tstruct { string; uint32 }\n\nhas 8 because it can stop immediately after the string pointer.\n\nBe aware that the most compact order is not always the most efficient.\nIn rare cases it may cause two variables each updated by its own goroutine\nto occupy the same CPU cache line, inducing a form of memory contention\nknown as \"false sharing\" that slows down both goroutines.\n",
- "Default": "false"
- },
- {
"Name": "\"fillreturns\"",
"Doc": "suggest fixes for errors due to an incorrect number of return values\n\nThis checker provides suggested fixes for type errors of the\ntype \"wrong number of return values (want %d, got %d)\". For example:\n\n\tfunc m() (int, string, *bool, error) {\n\t\treturn\n\t}\n\nwill turn into\n\n\tfunc m() (int, string, *bool, error) {\n\t\treturn 0, \"\", nil, nil\n\t}\n\nThis functionality is similar to https://github.com/sqs/goreturns.",
"Default": "true"
@@ -1361,12 +1356,6 @@
"Default": true
},
{
- "Name": "fieldalignment",
- "Doc": "find structs that would use less memory if their fields were sorted\n\nThis analyzer find structs that can be rearranged to use less memory, and provides\na suggested edit with the most compact order.\n\nNote that there are two different diagnostics reported. One checks struct size,\nand the other reports \"pointer bytes\" used. Pointer bytes is how many bytes of the\nobject that the garbage collector has to potentially scan for pointers, for example:\n\n\tstruct { uint32; string }\n\nhave 16 pointer bytes because the garbage collector has to scan up through the string's\ninner pointer.\n\n\tstruct { string; *uint32 }\n\nhas 24 pointer bytes because it has to scan further through the *uint32.\n\n\tstruct { string; uint32 }\n\nhas 8 because it can stop immediately after the string pointer.\n\nBe aware that the most compact order is not always the most efficient.\nIn rare cases it may cause two variables each updated by its own goroutine\nto occupy the same CPU cache line, inducing a form of memory contention\nknown as \"false sharing\" that slows down both goroutines.\n",
- "URL": "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/fieldalignment",
- "Default": false
- },
- {
"Name": "fillreturns",
"Doc": "suggest fixes for errors due to an incorrect number of return values\n\nThis checker provides suggested fixes for type errors of the\ntype \"wrong number of return values (want %d, got %d)\". For example:\n\n\tfunc m() (int, string, *bool, error) {\n\t\treturn\n\t}\n\nwill turn into\n\n\tfunc m() (int, string, *bool, error) {\n\t\treturn 0, \"\", nil, nil\n\t}\n\nThis functionality is similar to https://github.com/sqs/goreturns.",
"URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/fillreturns",
diff --git a/gopls/internal/settings/analysis.go b/gopls/internal/settings/analysis.go
index 5ae9e80..bbf3c75 100644
--- a/gopls/internal/settings/analysis.go
+++ b/gopls/internal/settings/analysis.go
@@ -20,7 +20,6 @@
"golang.org/x/tools/go/analysis/passes/defers"
"golang.org/x/tools/go/analysis/passes/directive"
"golang.org/x/tools/go/analysis/passes/errorsas"
- "golang.org/x/tools/go/analysis/passes/fieldalignment"
"golang.org/x/tools/go/analysis/passes/framepointer"
"golang.org/x/tools/go/analysis/passes/httpresponse"
"golang.org/x/tools/go/analysis/passes/ifaceassert"
@@ -184,9 +183,9 @@
{analyzer: embeddirective.Analyzer, enabled: true},
// disabled due to high false positives
- {analyzer: fieldalignment.Analyzer, enabled: false}, // never a bug
- {analyzer: shadow.Analyzer, enabled: false}, // very noisy
- {analyzer: useany.Analyzer, enabled: false}, // never a bug
+ {analyzer: shadow.Analyzer, enabled: false}, // very noisy
+ {analyzer: useany.Analyzer, enabled: false}, // never a bug
+ // fieldalignment is not even off-by-default; see #67762.
// "simplifiers": analyzers that offer mere style fixes
// gofmt -s suite:
diff --git a/gopls/internal/settings/settings.go b/gopls/internal/settings/settings.go
index d7ab4b1..ca0bf6a 100644
--- a/gopls/internal/settings/settings.go
+++ b/gopls/internal/settings/settings.go
@@ -1010,7 +1010,12 @@
DefinitionShortcut)
case "analyses":
- return setBoolMap(&o.Analyses, value)
+ if err := setBoolMap(&o.Analyses, value); err != nil {
+ return err
+ }
+ if o.Analyses["fieldalignment"] {
+ return deprecatedError("the 'fieldalignment' analyzer was removed in gopls/v0.17.0; instead, hover over struct fields to see size/offset information (https://go.dev/issue/66861)")
+ }
case "hints":
return setBoolMap(&o.Hints, value)
