[tools] go/analysis/passes/modernize: unsafefuncs: ptr+int => unsafe.Add

4 views
Skip to first unread message

Alan Donovan (Gerrit)

unread,
Dec 1, 2025, 4:30:25 PM (2 days ago) Dec 1
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Alan Donovan has uploaded the change for review

Commit message

go/analysis/passes/modernize: unsafefuncs: ptr+int => unsafe.Add

This CL adds a new modernizer (only to gopls for now) that
replaces unsafe pointer arithmetic by calls to helper functions
such as go1.17's unsafe.Add.

We really need some kind of template matcher like the one
in staticcheck, ideally integrated with cursors.

+ tests, docs.

For golang/go#76648
Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8

Change diff

diff --git a/go/analysis/passes/modernize/doc.go b/go/analysis/passes/modernize/doc.go
index 7469002..68cbc96 100644
--- a/go/analysis/passes/modernize/doc.go
+++ b/go/analysis/passes/modernize/doc.go
@@ -475,6 +475,33 @@
 This change is only suggested if the `cancel` function is not used
 for any other purpose.
 
+# Analyzer unsafefuncs
+
+unsafefuncs: replace unsafe pointer arithmetic with function calls
+
+The unsafefuncs analyzer simplifies pointer arithmetic expressions by
+replacing them will calls to helper functions such as unsafe.Add,
+added in Go 1.17.
+
+Example:
+
+	unsafe.Pointer(uintptr(ptr) + uintptr(n))
+
+where ptr is an unsafe.Pointer, is replaced by:
+
+	unsafe.Add(ptr, n)
+
+If the unsafe.Pointer was itself obtained from an ordinary *T pointer,
+and the result is converted to the same *T pointer type, the
+expression can be further simplified, as in this example wherfe ptr
+has type *T:
+
+	(*T)(unsafe.Pointer(uintptr(unsafe.Pointer(ptr)) + uintptr(n)))
+
+=>
+
+	unsafe.Add(ptr, n)
+
 # Analyzer waitgroup
 
 waitgroup: replace wg.Add(1)/go/wg.Done() with wg.Go
diff --git a/go/analysis/passes/modernize/modernize.go b/go/analysis/passes/modernize/modernize.go
index 013ce79..066984f 100644
--- a/go/analysis/passes/modernize/modernize.go
+++ b/go/analysis/passes/modernize/modernize.go
@@ -54,6 +54,7 @@
 	StringsBuilderAnalyzer,
 	TestingContextAnalyzer,
 	WaitGroupAnalyzer,
+	UnsafeFuncsAnalyzer,
 }
 
 // -- helpers --
diff --git a/go/analysis/passes/modernize/modernize_test.go b/go/analysis/passes/modernize/modernize_test.go
index ce4341b..dc91ade 100644
--- a/go/analysis/passes/modernize/modernize_test.go
+++ b/go/analysis/passes/modernize/modernize_test.go
@@ -109,6 +109,10 @@
 	RunWithSuggestedFixes(t, TestData(), modernize.TestingContextAnalyzer, "testingcontext")
 }
 
+func TestUnsafeFuncs(t *testing.T) {
+	RunWithSuggestedFixes(t, TestData(), modernize.UnsafeFuncsAnalyzer, "unsafefuncs")
+}
+
 func TestWaitGroup(t *testing.T) {
 	RunWithSuggestedFixes(t, TestData(), modernize.WaitGroupAnalyzer, "waitgroup")
 }
diff --git a/go/analysis/passes/modernize/testdata/src/unsafefuncs/unsafefuncs.go b/go/analysis/passes/modernize/testdata/src/unsafefuncs/unsafefuncs.go
new file mode 100644
index 0000000..3d5a67d
--- /dev/null
+++ b/go/analysis/passes/modernize/testdata/src/unsafefuncs/unsafefuncs.go
@@ -0,0 +1,23 @@
+package unsafefuncs
+
+import "unsafe"
+
+func _(ptr unsafe.Pointer) unsafe.Pointer {
+	return unsafe.Pointer(uintptr(ptr) + 1) // want `pointer \+ integer can be simplified using unsafe.Add`
+}
+
+type uP = unsafe.Pointer
+
+func _(ptr uP) uP {
+	return uP(uintptr(ptr) + 1) // want `pointer \+ integer can be simplified using unsafe.Add`
+}
+
+func _(ptr unsafe.Pointer, n int) unsafe.Pointer {
+	return unsafe.Pointer(uintptr(ptr) + uintptr(n)) // want `pointer \+ integer can be simplified using unsafe.Add`
+}
+
+type namedUP unsafe.Pointer
+
+func _(ptr namedUP) namedUP {
+	return namedUP(uintptr(ptr) + 1) // nope: Add does not accept named unsafe.Pointer types
+}
diff --git a/go/analysis/passes/modernize/testdata/src/unsafefuncs/unsafefuncs.go.golden b/go/analysis/passes/modernize/testdata/src/unsafefuncs/unsafefuncs.go.golden
new file mode 100644
index 0000000..fbad7c2
--- /dev/null
+++ b/go/analysis/passes/modernize/testdata/src/unsafefuncs/unsafefuncs.go.golden
@@ -0,0 +1,23 @@
+package unsafefuncs
+
+import "unsafe"
+
+func _(ptr unsafe.Pointer) unsafe.Pointer {
+	return unsafe.Add(ptr, 1) // want `pointer \+ integer can be simplified using unsafe.Add`
+}
+
+type uP = unsafe.Pointer
+
+func _(ptr uP) uP {
+	return unsafe.Add(ptr, 1) // want `pointer \+ integer can be simplified using unsafe.Add`
+}
+
+func _(ptr unsafe.Pointer, n int) unsafe.Pointer {
+	return unsafe.Add(ptr, n) // want `pointer \+ integer can be simplified using unsafe.Add`
+}
+
+type namedUP unsafe.Pointer
+
+func _(ptr namedUP) namedUP {
+	return namedUP(uintptr(ptr) + 1) // nope: Add does not accept named unsafe.Pointer types
+}
diff --git a/go/analysis/passes/modernize/unsafefuncs.go b/go/analysis/passes/modernize/unsafefuncs.go
new file mode 100644
index 0000000..302156c
--- /dev/null
+++ b/go/analysis/passes/modernize/unsafefuncs.go
@@ -0,0 +1,221 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package modernize
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/edge"
+	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/internal/analysis/analyzerutil"
+	"golang.org/x/tools/internal/astutil"
+	"golang.org/x/tools/internal/refactor"
+	"golang.org/x/tools/internal/typesinternal"
+	"golang.org/x/tools/internal/versions"
+)
+
+// TODO(adonovan): also support:
+//
+// func String(ptr *byte, len IntegerType) string
+// func StringData(str string) *byte
+// func Slice(ptr *ArbitraryType, len IntegerType) []ArbitraryType
+// func SliceData(slice []ArbitraryType) *ArbitraryType
+
+var UnsafeFuncsAnalyzer = &analysis.Analyzer{
+	Name:     "unsafefuncs",
+	Doc:      analyzerutil.MustExtractDoc(doc, "unsafefuncs"),
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      unsafefuncs,
+	URL:      "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#unsafefuncs",
+}
+
+func unsafefuncs(pass *analysis.Pass) (any, error) {
+	// Short circuit if the package doesn't use unsafe.
+	// (In theory one could use some imported alias of unsafe.Pointer,
+	// but let's ignore that.)
+	if !typesinternal.Imports(pass.Pkg, "unsafe") {
+		return nil, nil
+	}
+
+	var (
+		inspect        = pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+		info           = pass.TypesInfo
+		tUnsafePointer = types.Typ[types.UnsafePointer]
+	)
+
+	isInteger := func(t types.Type) bool {
+		basic, ok := t.Underlying().(*types.Basic)
+		return ok && basic.Info()&types.IsInteger != 0
+	}
+
+	// isConversion reports whether e is a conversion T(x).
+	// If so, it returns T and x.
+	isConversion := func(curExpr inspector.Cursor) (t types.Type, x inspector.Cursor) {
+		e := curExpr.Node().(ast.Expr)
+		if conv, ok := ast.Unparen(e).(*ast.CallExpr); ok && len(conv.Args) == 1 {
+			if tv := pass.TypesInfo.Types[conv.Fun]; tv.IsType() {
+				return tv.Type, curExpr.ChildAt(edge.CallExpr_Args, 0)
+			}
+		}
+		return
+	}
+
+	// The general form is where ptr and the result are of type unsafe.Pointer:
+	//
+	// 	unsafe.Pointer(uintptr(ptr) + uintptr(n))
+	// =>
+	// 	unsafe.Add(ptr, n)
+	//
+	// Variant: ptr is itself converted from *T, and the sum is converted back to *T:
+	//
+	// 	(*T)(unsafe.Pointer(uintptr(unsafe.Pointer(ptr)) + uintptr(n)))
+	// =>
+	// 	unsafe.Add(ptr, n)
+
+	// Search for 'unsafe.Pointer(uintptr + uintptr)'
+	// where the left operand was converted from a pointer.
+	//
+	// (Start from sum, not conversion, as it is not
+	// uncommon to use a local type alias for unsafe.Pointer.)
+	for curSum := range inspect.Root().Preorder((*ast.BinaryExpr)(nil)) {
+		if sum, ok := curSum.Node().(*ast.BinaryExpr); ok &&
+			sum.Op == token.ADD &&
+			types.Identical(info.TypeOf(sum.X), types.Typ[types.Uintptr]) &&
+			astutil.IsChildOf(curSum, edge.CallExpr_Args) {
+			// Have: sum ≡ T(x:...uintptr... + y:...uintptr...)
+			curX := curSum.ChildAt(edge.BinaryExpr_X, -1)
+			curY := curSum.ChildAt(edge.BinaryExpr_Y, -1)
+
+			// Is sum converted to unsafe.Pointer?
+			curResult := curSum.Parent()
+			if t, _ := isConversion(curResult); !(t != nil && types.Identical(t, tUnsafePointer)) {
+				continue
+			}
+			// Have: result ≡ unsafe.Pointer(x:...uintptr... + y:...uintptr...)
+
+			// Is sum.x converted from unsafe.Pointer?
+			_, curPtr := isConversion(curX)
+			ptr := curPtr.Node().(ast.Expr)
+			if !(astutil.CursorValid(curPtr) && types.Identical(info.TypeOf(ptr), tUnsafePointer)) {
+				continue
+			}
+			// Have: result ≡ unsafe.Pointer(x:uintptr(...unsafe.Pointer...) + y:...uintptr...)
+
+			file := astutil.EnclosingFile(curSum)
+			if !analyzerutil.FileUsesGoVersion(pass, file, versions.Go1_17) {
+				continue // unsafe.Add not available in this file
+			}
+
+			// import "unsafe"
+			unsafedot, edits := refactor.AddImport(info, file, "unsafe", "unsafe", "Add", sum.Pos())
+
+			// unsafe.Pointer(x + y)
+			// ---------------     -
+			//                x + y
+			edits = append(edits, deleteConv(curResult)...)
+
+			// uintptr   (ptr) + offset
+			// -----------   ----      -
+			// unsafe.Add(ptr,   offset)
+			edits = append(edits, []analysis.TextEdit{
+				{
+					Pos:     sum.Pos(),
+					End:     ptr.Pos(),
+					NewText: fmt.Appendf(nil, "%sAdd(", unsafedot),
+				},
+				{
+					Pos:     ptr.End(),
+					End:     sum.Y.Pos(),
+					NewText: []byte(", "),
+				},
+				{
+					Pos:     sum.Y.End(),
+					End:     sum.Y.End(),
+					NewText: []byte(")"),
+				},
+			}...)
+
+			// Variant: ptr/result are converted from/to some pointer type *T.
+			// Discard the matched inner and outer conversions.
+			//
+			// 	(*T)(unsafe.Pointer(uintptr(unsafe.Pointer(ptr)) + ...uintptr...))
+			//      ----                        ---------------   -                  -
+			// 	     unsafe.Pointer(uintptr(               ptr ) + ...uintptr...)
+			//
+			if tInner, _ := isConversion(curPtr); tInner != nil &&
+				is[*types.Pointer](tInner.Underlying()) &&
+				astutil.IsChildOf(curResult, edge.CallExpr_Args) {
+				curOuterConv := curResult.Parent()
+				if tOuter, _ := isConversion(curOuterConv); tOuter != nil &&
+					types.Identical(tOuter, tInner) {
+					edits = append(edits, deleteConv(curOuterConv)...)
+					edits = append(edits, deleteConv(curPtr)...)
+				}
+			}
+
+			// Variant: sum.y operand was converted from another integer type.
+			// Discard the conversion, as Add is generic over integers.
+			if t, _ := isConversion(curY); t != nil && isInteger(t) {
+				edits = append(edits, deleteConv(curY)...)
+			}
+
+			pass.Report(analysis.Diagnostic{
+				Pos:     sum.Pos(),
+				End:     sum.End(),
+				Message: "pointer + integer can be simplified using unsafe.Add",
+				SuggestedFixes: []analysis.SuggestedFix{{
+					Message:   "Simplify pointer addition using unsafe.Add",
+					TextEdits: edits,
+				}},
+			})
+		}
+	}
+
+	return nil, nil
+}
+
+// deleteConv returns edits for changing T(x) to x, respecting precedence.
+func deleteConv(cur inspector.Cursor) []analysis.TextEdit {
+	conv := cur.Node().(*ast.CallExpr)
+
+	usesPrec := func(n ast.Node) bool {
+		switch n.(type) {
+		case *ast.BinaryExpr, *ast.UnaryExpr:
+			return true
+		}
+		return false
+	}
+
+	// Be careful not to change precedence of e.g. T(1+2) * 3.
+	// TODO(adonovan): refine this.
+	if usesPrec(cur.Parent().Node()) && usesPrec(conv.Args[0]) {
+		// T(x+y) * z
+		// -
+		//  (x+y) * z
+		return []analysis.TextEdit{{
+			Pos: conv.Fun.Pos(),
+			End: conv.Fun.End(),
+		}}
+	}
+
+	// T(x)
+	// -- -
+	//   x
+	return []analysis.TextEdit{
+		{
+			Pos: conv.Pos(),
+			End: conv.Args[0].Pos(),
+		},
+		{
+			Pos: conv.Args[0].End(),
+			End: conv.End(),
+		},
+	}
+}
diff --git a/go/types/typeutil/callee.go b/go/types/typeutil/callee.go
index 5f10f56..3d24a8c 100644
--- a/go/types/typeutil/callee.go
+++ b/go/types/typeutil/callee.go
@@ -12,6 +12,7 @@
 
 // Callee returns the named target of a function call, if any:
 // a function, method, builtin, or variable.
+// It returns nil for a T(x) conversion.
 //
 // Functions and methods may potentially have type parameters.
 //
diff --git a/internal/versions/features.go b/internal/versions/features.go
index a5f4e32..cdd36c3 100644
--- a/internal/versions/features.go
+++ b/internal/versions/features.go
@@ -9,6 +9,7 @@
 
 // named constants, to avoid misspelling
 const (
+	Go1_17 = "go1.17"
 	Go1_18 = "go1.18"
 	Go1_19 = "go1.19"
 	Go1_20 = "go1.20"

Change information

Files:
  • M go/analysis/passes/modernize/doc.go
  • M go/analysis/passes/modernize/modernize.go
  • M go/analysis/passes/modernize/modernize_test.go
  • A go/analysis/passes/modernize/testdata/src/unsafefuncs/unsafefuncs.go
  • A go/analysis/passes/modernize/testdata/src/unsafefuncs/unsafefuncs.go.golden
  • A go/analysis/passes/modernize/unsafefuncs.go
  • M go/types/typeutil/callee.go
  • M internal/versions/features.go
Change size: L
Delta: 8 files changed, 301 insertions(+), 0 deletions(-)
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newchange
Gerrit-Project: tools
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
Gerrit-Change-Number: 725680
Gerrit-PatchSet: 1
Gerrit-Owner: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

Alan Donovan (Gerrit)

unread,
Dec 1, 2025, 4:33:28 PM (2 days ago) Dec 1
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan

Alan Donovan uploaded new patchset

Alan Donovan uploaded patch set #2 to this change.
Open in Gerrit

Related details

Attention is currently required from:
  • Alan Donovan
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newpatchset
Gerrit-Project: tools
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
Gerrit-Change-Number: 725680
Gerrit-PatchSet: 2
Gerrit-Owner: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
Gerrit-Attention: Alan Donovan <adon...@google.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

Alan Donovan (Gerrit)

unread,
Dec 1, 2025, 4:48:19 PM (2 days ago) Dec 1
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Alan Donovan

Alan Donovan uploaded new patchset

Alan Donovan uploaded patch set #3 to this change.
Following approvals got outdated and were removed:
  • TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
Open in Gerrit

Related details

Attention is currently required from:
  • Alan Donovan
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newpatchset
Gerrit-Project: tools
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
Gerrit-Change-Number: 725680
Gerrit-PatchSet: 3
Gerrit-Owner: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-CC: Michael Matloob <mat...@golang.org>
Gerrit-CC: Robert Findley <rfin...@google.com>
Gerrit-Attention: Alan Donovan <adon...@google.com>
unsatisfied_requirement
satisfied_requirement
open
diffy

David Chase (Gerrit)

unread,
Dec 1, 2025, 5:06:45 PM (2 days ago) Dec 1
to Alan Donovan, goph...@pubsubhelper.golang.org, Robert Findley, Go LUCI, Michael Matloob, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Robert Findley

David Chase added 1 comment

Patchset-level comments
File-level comment, Patchset 3 (Latest):
David Chase . resolved

Nice. I need to get you those other unsafe idioms, but it was helpful to read these to see how it's done. Not sure I am qualified to +2 it.

Open in Gerrit

Related details

Attention is currently required from:
  • Alan Donovan
  • Robert Findley
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: tools
Gerrit-Branch: master
Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
Gerrit-Change-Number: 725680
Gerrit-PatchSet: 3
Gerrit-Owner: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Alan Donovan <adon...@google.com>
Gerrit-Reviewer: Robert Findley <rfin...@google.com>
Gerrit-CC: David Chase <drc...@google.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-CC: Michael Matloob <mat...@golang.org>
Gerrit-Attention: Alan Donovan <adon...@google.com>
Gerrit-Attention: Robert Findley <rfin...@google.com>
Gerrit-Comment-Date: Mon, 01 Dec 2025 22:06:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
unsatisfied_requirement
satisfied_requirement
open
diffy

Robert Findley (Gerrit)

unread,
Dec 1, 2025, 6:57:31 PM (2 days ago) Dec 1
to Alan Donovan, goph...@pubsubhelper.golang.org, Go LUCI, David Chase, Michael Matloob, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Alan Donovan

Robert Findley added 1 comment

Patchset-level comments
Robert Findley . resolved

Sorry, I'm really not going to have time to review this thoroughly 😞

Open in Gerrit

Related details

Attention is currently required from:
  • Alan Donovan
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
    Gerrit-Change-Number: 725680
    Gerrit-PatchSet: 3
    Gerrit-Owner: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Robert Findley <rfin...@google.com>
    Gerrit-CC: David Chase <drc...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: Michael Matloob <mat...@golang.org>
    Gerrit-Attention: Alan Donovan <adon...@google.com>
    Gerrit-Comment-Date: Mon, 01 Dec 2025 23:57:27 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Daniel Morsing (Gerrit)

    unread,
    Dec 2, 2025, 4:20:11 AM (23 hours ago) Dec 2
    to Alan Donovan, goph...@pubsubhelper.golang.org, Robert Findley, Go LUCI, David Chase, Michael Matloob, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Alan Donovan

    Daniel Morsing voted and added 2 comments

    Votes added by Daniel Morsing

    Code-Review+1

    2 comments

    Patchset-level comments
    Daniel Morsing . resolved

    LGTM with a tiny nit

    File go/analysis/passes/modernize/unsafefuncs.go
    Line 96, Patchset 3 (Latest): // Is sum converted to unsafe.Pointer?
    Daniel Morsing . resolved

    Because the type conversions can be a bit clunky, it's quite common to introduce temporary variables for readability.

    ```
    y := uintptr(unsafe.Pointer(ptr)) + uintptr(someval)
    return (*sometype)(unsafe.Pointer(y))
    ```

    I suspect we could catch some more cases if you tested whether the expression is assigned to a variable that is used exactly once in a conversion to unsafe.Pointer, but that's probably out of scope for now. Maybe a TODO?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Alan Donovan
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
    Gerrit-Change-Number: 725680
    Gerrit-PatchSet: 3
    Gerrit-Owner: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Daniel Morsing <daniel....@gmail.com>
    Gerrit-CC: David Chase <drc...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: Michael Matloob <mat...@golang.org>
    Gerrit-CC: Robert Findley <rfin...@google.com>
    Gerrit-Attention: Alan Donovan <adon...@google.com>
    Gerrit-Comment-Date: Tue, 02 Dec 2025 09:20:02 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Dominik Honnef (Gerrit)

    unread,
    Dec 2, 2025, 11:06:26 AM (17 hours ago) Dec 2
    to Alan Donovan, goph...@pubsubhelper.golang.org, Dominik Honnef, Madeline Kalil, Daniel Morsing, Robert Findley, Go LUCI, David Chase, Michael Matloob, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Alan Donovan, Daniel Morsing and Madeline Kalil

    Dominik Honnef added 1 comment

    File go/analysis/passes/modernize/unsafefuncs.go
    Line 96, Patchset 3 (Latest): // Is sum converted to unsafe.Pointer?
    Daniel Morsing . resolved

    Because the type conversions can be a bit clunky, it's quite common to introduce temporary variables for readability.

    ```
    y := uintptr(unsafe.Pointer(ptr)) + uintptr(someval)
    return (*sometype)(unsafe.Pointer(y))
    ```

    I suspect we could catch some more cases if you tested whether the expression is assigned to a variable that is used exactly once in a conversion to unsafe.Pointer, but that's probably out of scope for now. Maybe a TODO?

    Dominik Honnef

    That temporary variable is not actually valid. Grep https://pkg.go.dev/unsafe for "Note that both conversions must appear in the same expression"

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Alan Donovan
    • Daniel Morsing
    • Madeline Kalil
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
    Gerrit-Change-Number: 725680
    Gerrit-PatchSet: 3
    Gerrit-Owner: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Daniel Morsing <daniel....@gmail.com>
    Gerrit-Reviewer: Madeline Kalil <mka...@google.com>
    Gerrit-CC: David Chase <drc...@google.com>
    Gerrit-CC: Dominik Honnef <dom...@honnef.co>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: Michael Matloob <mat...@golang.org>
    Gerrit-CC: Robert Findley <rfin...@google.com>
    Gerrit-Attention: Madeline Kalil <mka...@google.com>
    Gerrit-Attention: Alan Donovan <adon...@google.com>
    Gerrit-Attention: Daniel Morsing <daniel....@gmail.com>
    Gerrit-Comment-Date: Tue, 02 Dec 2025 16:06:18 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Daniel Morsing <daniel....@gmail.com>
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Daniel Morsing (Gerrit)

    unread,
    Dec 2, 2025, 12:07:11 PM (16 hours ago) Dec 2
    to Alan Donovan, goph...@pubsubhelper.golang.org, Dominik Honnef, Madeline Kalil, Robert Findley, Go LUCI, David Chase, Michael Matloob, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Alan Donovan, Dominik Honnef and Madeline Kalil

    Daniel Morsing added 1 comment

    File go/analysis/passes/modernize/unsafefuncs.go
    Line 96, Patchset 3 (Latest): // Is sum converted to unsafe.Pointer?
    Daniel Morsing . resolved

    Because the type conversions can be a bit clunky, it's quite common to introduce temporary variables for readability.

    ```
    y := uintptr(unsafe.Pointer(ptr)) + uintptr(someval)
    return (*sometype)(unsafe.Pointer(y))
    ```

    I suspect we could catch some more cases if you tested whether the expression is assigned to a variable that is used exactly once in a conversion to unsafe.Pointer, but that's probably out of scope for now. Maybe a TODO?

    Dominik Honnef

    That temporary variable is not actually valid. Grep https://pkg.go.dev/unsafe for "Note that both conversions must appear in the same expression"

    Daniel Morsing

    Well that's egg on my face. I've been using unsafe.Pointer so long, but it's been a while since I consulted the nitty gritty of the documentation. I have definitely seen (and written) that idiom around before, so I rescind my comment and present instead #76657

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Alan Donovan
    • Dominik Honnef
    • Madeline Kalil
    Gerrit-Attention: Dominik Honnef <dom...@honnef.co>
    Gerrit-Comment-Date: Tue, 02 Dec 2025 17:07:03 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Dominik Honnef <dom...@honnef.co>
    Comment-In-Reply-To: Daniel Morsing <daniel....@gmail.com>
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Alan Donovan (Gerrit)

    unread,
    Dec 2, 2025, 4:19:55 PM (11 hours ago) Dec 2
    to goph...@pubsubhelper.golang.org, Dominik Honnef, Madeline Kalil, Daniel Morsing, Robert Findley, Go LUCI, David Chase, Michael Matloob, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Daniel Morsing, Dominik Honnef and Madeline Kalil

    Alan Donovan added 1 comment

    File go/analysis/passes/modernize/unsafefuncs.go
    Line 96, Patchset 3 (Latest): // Is sum converted to unsafe.Pointer?
    Daniel Morsing . resolved

    Because the type conversions can be a bit clunky, it's quite common to introduce temporary variables for readability.

    ```
    y := uintptr(unsafe.Pointer(ptr)) + uintptr(someval)
    return (*sometype)(unsafe.Pointer(y))
    ```

    I suspect we could catch some more cases if you tested whether the expression is assigned to a variable that is used exactly once in a conversion to unsafe.Pointer, but that's probably out of scope for now. Maybe a TODO?

    Dominik Honnef

    That temporary variable is not actually valid. Grep https://pkg.go.dev/unsafe for "Note that both conversions must appear in the same expression"

    Daniel Morsing

    Well that's egg on my face. I've been using unsafe.Pointer so long, but it's been a while since I consulted the nitty gritty of the documentation. I have definitely seen (and written) that idiom around before, so I rescind my comment and present instead #76657

    Alan Donovan

    Dominik makes a good point. Probably the right thing is for this mistake to be detected by the existing `unsafeptr` analyzer. (I'm surprised it doesn't already check for it.) I've filed https://github.com/golang/go/issues/76660.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Daniel Morsing
    • Dominik Honnef
    • Madeline Kalil
    Gerrit-Attention: Dominik Honnef <dom...@honnef.co>
    Gerrit-Attention: Daniel Morsing <daniel....@gmail.com>
    Gerrit-Comment-Date: Tue, 02 Dec 2025 21:19:51 +0000
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Madeline Kalil (Gerrit)

    unread,
    Dec 2, 2025, 4:28:25 PM (11 hours ago) Dec 2
    to Alan Donovan, goph...@pubsubhelper.golang.org, Dominik Honnef, Daniel Morsing, Robert Findley, Go LUCI, David Chase, Michael Matloob, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Daniel Morsing and Dominik Honnef

    Madeline Kalil voted and added 1 comment

    Votes added by Madeline Kalil

    Code-Review+2

    1 comment

    File go/analysis/passes/modernize/unsafefuncs.go
    Line 148, Patchset 3 (Latest): // Variant: sum.y operand was converted from another integer type.
    Madeline Kalil . unresolved

    Could you please add an example or a test case illustrating this variant?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Daniel Morsing
    • Dominik Honnef
    Submit Requirements:
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement satisfiedReview-Enforcement
    • requirement satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: tools
    Gerrit-Branch: master
    Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
    Gerrit-Change-Number: 725680
    Gerrit-PatchSet: 3
    Gerrit-Owner: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Alan Donovan <adon...@google.com>
    Gerrit-Reviewer: Daniel Morsing <daniel....@gmail.com>
    Gerrit-Reviewer: Madeline Kalil <mka...@google.com>
    Gerrit-CC: David Chase <drc...@google.com>
    Gerrit-CC: Dominik Honnef <dom...@honnef.co>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: Michael Matloob <mat...@golang.org>
    Gerrit-CC: Robert Findley <rfin...@google.com>
    Gerrit-Attention: Dominik Honnef <dom...@honnef.co>
    Gerrit-Attention: Daniel Morsing <daniel....@gmail.com>
    Gerrit-Comment-Date: Tue, 02 Dec 2025 21:28:22 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Alan Donovan (Gerrit)

    unread,
    Dec 2, 2025, 9:18:30 PM (6 hours ago) Dec 2
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
    Attention needed from Daniel Morsing and Dominik Honnef

    Alan Donovan uploaded new patchset

    Alan Donovan uploaded patch set #4 to this change.
    Following approvals got outdated and were removed:
    • TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Daniel Morsing
    • Dominik Honnef
    Submit Requirements:
      • requirement satisfiedCode-Review
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement satisfiedReview-Enforcement
      • requirement is not satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: newpatchset
      Gerrit-Project: tools
      Gerrit-Branch: master
      Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
      Gerrit-Change-Number: 725680
      Gerrit-PatchSet: 4
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Alan Donovan (Gerrit)

      unread,
      Dec 2, 2025, 9:19:39 PM (6 hours ago) Dec 2
      to goph...@pubsubhelper.golang.org, Madeline Kalil, Dominik Honnef, Daniel Morsing, Robert Findley, Go LUCI, David Chase, Michael Matloob, Gopher Robot, golang-co...@googlegroups.com
      Attention needed from Daniel Morsing and Dominik Honnef

      Alan Donovan voted and added 1 comment

      Votes added by Alan Donovan

      Auto-Submit+1
      Commit-Queue+1

      1 comment

      File go/analysis/passes/modernize/unsafefuncs.go
      Line 148, Patchset 3: // Variant: sum.y operand was converted from another integer type.
      Madeline Kalil . resolved

      Could you please add an example or a test case illustrating this variant?

      Alan Donovan

      Sure; done. :)

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Daniel Morsing
      • Dominik Honnef
      Submit Requirements:
      • requirement satisfiedCode-Review
      • requirement satisfiedNo-Unresolved-Comments
      • requirement satisfiedReview-Enforcement
      • requirement is not satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: tools
      Gerrit-Branch: master
      Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
      Gerrit-Change-Number: 725680
      Gerrit-PatchSet: 3
      Gerrit-Owner: Alan Donovan <adon...@google.com>
      Gerrit-Reviewer: Alan Donovan <adon...@google.com>
      Gerrit-Reviewer: Daniel Morsing <daniel....@gmail.com>
      Gerrit-Reviewer: Madeline Kalil <mka...@google.com>
      Gerrit-CC: David Chase <drc...@google.com>
      Gerrit-CC: Dominik Honnef <dom...@honnef.co>
      Gerrit-CC: Gopher Robot <go...@golang.org>
      Gerrit-CC: Michael Matloob <mat...@golang.org>
      Gerrit-CC: Robert Findley <rfin...@google.com>
      Gerrit-Attention: Dominik Honnef <dom...@honnef.co>
      Gerrit-Attention: Daniel Morsing <daniel....@gmail.com>
      Gerrit-Comment-Date: Wed, 03 Dec 2025 02:19:35 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      Comment-In-Reply-To: Madeline Kalil <mka...@google.com>
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Alan Donovan (Gerrit)

      unread,
      Dec 2, 2025, 9:58:14 PM (6 hours ago) Dec 2
      to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Madeline Kalil, Dominik Honnef, Daniel Morsing, Robert Findley, David Chase, Michael Matloob, Gopher Robot, golang-co...@googlegroups.com

      Alan Donovan submitted the change with unreviewed changes

      Unreviewed changes

      3 is the latest approved patch-set.
      The change was submitted with unreviewed changes in the following files:

      ```
      The name of the file: go/analysis/passes/modernize/unsafefuncs.go
      Insertions: 4, Deletions: 0.

      @@ -147,6 +147,10 @@

       
       			// Variant: sum.y operand was converted from another integer type.
       			// Discard the conversion, as Add is generic over integers.
      +			//
      +			// e.g. unsafe.Pointer(uintptr(ptr) + uintptr(len(s)))
      +			//                                    --------      -
      +			//      unsafe.Add    (        ptr,           len(s))

       			if t, _ := isConversion(curY); t != nil && isInteger(t) {
       				edits = append(edits, deleteConv(curY)...)
       			}
      ```

      Change information

      Commit message:
      go/analysis/passes/modernize: unsafefuncs: ptr+int => unsafe.Add

      This CL adds a new modernizer (only to gopls for now) that
      replaces unsafe pointer arithmetic by calls to helper functions
      such as go1.17's unsafe.Add.

      We really need some kind of template matcher like the one
      in staticcheck, ideally integrated with cursors.

      + tests, docs.

      For golang/go#76648
      Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
      Reviewed-by: Daniel Morsing <daniel....@gmail.com>
      Reviewed-by: Madeline Kalil <mka...@google.com>
      Files:
        • M go/analysis/passes/modernize/doc.go
        • M go/analysis/passes/modernize/modernize.go
        • M go/analysis/passes/modernize/modernize_test.go
        • A go/analysis/passes/modernize/testdata/src/unsafefuncs/unsafefuncs.go
        • A go/analysis/passes/modernize/testdata/src/unsafefuncs/unsafefuncs.go.golden
        • A go/analysis/passes/modernize/unsafefuncs.go
        • M go/types/typeutil/callee.go
        • M gopls/doc/analyzers.md
        • M gopls/internal/doc/api.json
        • M gopls/internal/settings/analysis.go
        • M internal/goplsexport/export.go
        • M internal/versions/features.go
        Change size: L
        Delta: 12 files changed, 319 insertions(+), 0 deletions(-)
        Branch: refs/heads/master
        Submit Requirements:
        • requirement satisfiedCode-Review: +1 by Daniel Morsing, +2 by Madeline Kalil
        • requirement satisfiedTryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
        Open in Gerrit
        Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
        Gerrit-MessageType: merged
        Gerrit-Project: tools
        Gerrit-Branch: master
        Gerrit-Change-Id: Ia3e7d603efd8a01e723370b485ab83b6838522b8
        Gerrit-Change-Number: 725680
        Gerrit-PatchSet: 5
        open
        diffy
        satisfied_requirement
        Reply all
        Reply to author
        Forward
        0 new messages