[インデックス 13880] ファイルの概要

このコミットは、Go言語の静的解析ツールである go vet の rangeloop チェッカーにおけるバグ修正です。具体的には、src/cmd/vet/rangeloop.go ファイルが変更され、range ループのキーが識別子ではない場合に発生していたパニック（panic）が修正されました。

コミット

• コミットハッシュ: 5a93fea08e7c9cbc4ed5ab7ba161b3e078497fb3
• Author: David Symonds dsymonds@golang.org
• Date: Thu Sep 20 08:12:47 2012 +1000

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/5a93fea08e7c9cbc4ed5ab7ba161b3e078497fb3

元コミット内容

vet: fix rangeloop.

In a range loop, the presence of a value implies the presence of a key.
However, the presence of a value as an *ast.Ident does not imply that
the key is also an *ast.Ident, thus leading to a panic any time the
two argument form is used where the key is not an identifier.

R=golang-dev, adg, r
CC=golang-dev
https://golang.org/cl/6540045

変更の背景

go vet ツールには、range ループ内で宣言された変数が、そのループ内で定義されたクロージャ（無名関数）によって「囲まれる」（キャプチャされる）ことによって発生する潜在的なバグを検出する rangeloop チェッカーが含まれています。これはGo言語における一般的な落とし穴の一つで、ループが反復を終えた後にクロージャが実行されると、ループ変数が最後に代入された値のみを参照してしまうという問題です。go vet はこのパターンを検出し、「range variable enclosed by function」という警告を出します。

このコミット以前の rangeloop チェッカーには、特定の条件下でパニックが発生するというバグがありました。具体的には、range ループがキーと値の両方を受け取る形式（for key, value := range collection）で使用され、かつキーが単純な識別子（*ast.Ident）ではない場合（例: for x[0], f = range s のように、キーが配列の要素や構造体のフィールドなど、より複雑な式である場合）に問題が発生しました。

元のコードは、range ループの値が *ast.Ident である場合、キーもまた *ast.Ident であると誤って仮定していました。この仮定が崩れると、キーを表す抽象構文木 (AST) ノードが nil にもかかわらず、その Obj フィールドにアクセスしようとして nil ポインタデリファレンスが発生し、go vet 実行時にパニックを引き起こしていました。このコミットは、この誤った仮定を修正し、パニックを回避することを目的としています。

前提知識の解説

Go言語の range ループ

Go言語の for ... range ループは、スライス、配列、文字列、マップ、チャネルなどのコレクションを反復処理するための強力な構文です。 基本的な形式は以下の通りです。

1. 値のみ: for value := range collection { ... }
   • インデックスやキーが不要な場合に使用します。
2. キーと値: for key, value := range collection { ... }
   • インデックスやキーと値の両方が必要な場合に使用します。

go vet の rangeloop チェッカーが対象とするのは、この range ループ内で宣言された key や value 変数が、ループ内で定義されたクロージャにキャプチャされるケースです。

package main

import "fmt"

func main() {
    nums := []int{1, 2, 3}
    var funcs []func()

    for i, v := range nums { // iとvはループごとに再利用される単一の変数
        funcs = append(funcs, func() {
            fmt.Printf("Index: %d, Value: %d\n", i, v) // ここでiとvがキャプチャされる
        })
    }

    // ループ終了後、iとvは最後の値（i=2, v=3）を持つ
    for _, f := range funcs {
        f() // 全てのクロージャが最後のiとvの値を参照してしまう
    }
    // 期待される出力:
    // Index: 0, Value: 1
    // Index: 1, Value: 2
    // Index: 2, Value: 3
    // 実際の出力:
    // Index: 2, Value: 3
    // Index: 2, Value: 3
    // Index: 2, Value: 3

    // 正しいキャプチャ方法（ループ変数をクロージャの引数として渡すか、新しい変数にコピーする）
    funcs = nil
    for i, v := range nums {
        i_copy, v_copy := i, v // 新しい変数にコピー
        funcs = append(funcs, func() {
            fmt.Printf("Index: %d, Value: %d\n", i_copy, v_copy)
        })
    }
    for _, f := range funcs {
        f()
    }
}

go vet は上記の最初の for ループのようなコードに対して警告を発します。

go vet ツール

go vet は、Goプログラムの疑わしい構成要素（バグの可能性が高いがコンパイルエラーにはならないもの）を検出するための静的解析ツールです。例えば、フォーマット文字列の不一致、到達不能なコード、ロックの誤用、そしてこのコミットが関連する range ループ変数のクロージャによるキャプチャなどが挙げられます。開発者が潜在的な問題を早期に発見し、より堅牢なコードを書くのに役立ちます。

go/ast パッケージ

go/ast パッケージは、Go言語のソースコードを抽象構文木 (Abstract Syntax Tree, AST) として表現するためのデータ構造を提供します。コンパイラや静的解析ツール（go vet など）は、このASTを解析してコードの意味を理解し、様々な処理を行います。

• ast.Node: AST内のすべてのノードが実装するインターフェース。
• ast.Ident: 識別子（変数名、関数名など）を表すノード。
• ast.RangeStmt: for ... range 文を表すノード。
• ast.Object: 識別子が参照するオブジェクト（変数、型、関数など）を表す。ast.Ident の Obj フィールドを通じてアクセスされます。
• ast.Inspect: ASTを再帰的に走査するためのユーティリティ関数。特定のノードタイプを検索したり、AST全体をウォークしたりする際に使用されます。

panic

panic はGo言語におけるランタイムエラーの一種で、プログラムの実行を即座に停止させます。通常、回復不可能なエラーやプログラマの論理的な誤りによって発生します。このコミットで修正された問題は、go vet ツール自体がパニックを起こすというものでした。

技術的詳細

このコミットの核心は、go vet の src/cmd/vet/rangeloop.go ファイル内の checkRangeLoop 関数にあります。この関数は、range ループのASTノード (*ast.RangeStmt) を受け取り、そのループ内で定義されたクロージャが range 変数を誤ってキャプチャしていないかを検査します。

元のコードでは、ast.Inspect を使用して range ループのボディ（lit.Body）を走査し、各ノード n が *ast.Ident（識別子）であるかどうかをチェックしていました。

// 変更前 (簡略化)
ast.Inspect(lit.Body, func(n ast.Node) bool {
    if n, ok := n.(*ast.Ident); ok && n.Obj != nil && (n.Obj == key.Obj || n.Obj == val.Obj) {
        f.Warn(n.Pos(), "range variable", n.Name, "enclosed by function")
    }
    return true
})

問題は、range ループのキー (key) が常に *ast.Ident であるとは限らない点にありました。例えば、for x[0], f = range s のような構文では、キーは x[0] という式であり、これは *ast.Ident ではありません。しかし、val（値）は f という *ast.Ident です。

元のコードでは、n が *ast.Ident であり、かつ n.Obj が key.Obj または val.Obj と一致するかどうかをチェックしていました。ここで、もし key が *ast.Ident ではない場合、key 変数自体は nil になります（ast.RangeStmt の Key フィールドは ast.Expr 型であり、*ast.Ident ではない式も格納できるため）。key が nil であるにもかかわらず key.Obj にアクセスしようとすると、nil ポインタデリファレンスが発生し、go vet がパニックを起こしていました。

このコミットによる修正は、この nil ポインタデリファレンスを防ぐためのものです。

1. まず、走査中のノード n を *ast.Ident に型アサートし、その結果を id と ok で受け取ります。
2. !ok || id.Obj == nil のチェックを追加し、n が識別子ではない場合や、識別子であってもそれが参照するオブジェクト (Obj) が nil の場合は、それ以上処理せずに早期に return true します。これにより、無効なノードに対する後続のアクセスを防ぎます。
3. 最も重要な変更は、key != nil のチェックを (n.Obj == key.Obj || n.Obj == val.Obj) の条件に追加したことです。これにより、key が nil である場合に key.Obj へのアクセスを試みることを防ぎ、パニックを回避します。val についても同様に val != nil のチェックが追加されていますが、元の問題は key 側にありました。

この修正により、go vet は range ループのキーが複雑な式である場合でも、安全に解析を続行できるようになりました。コミットに含まれるテストケース for x[0], f = range s は、まさにこのパニックを引き起こしていたシナリオを再現し、修正が正しく機能することを確認しています。

コアとなるコードの変更箇所

src/cmd/vet/rangeloop.go ファイルの checkRangeLoop 関数内の ast.Inspect のコールバック関数が変更されています。

--- a/src/cmd/vet/rangeloop.go
+++ b/src/cmd/vet/rangeloop.go
@@ -53,8 +53,12 @@ func checkRangeLoop(f *File, n *ast.RangeStmt) {
 	return
 }
 ast.Inspect(lit.Body, func(n ast.Node) bool {
-		if n, ok := n.(*ast.Ident); ok && n.Obj != nil && (n.Obj == key.Obj || n.Obj == val.Obj) {
-			f.Warn(n.Pos(), "range variable", n.Name, "enclosed by function")
+		id, ok := n.(*ast.Ident) // ノードをast.Identに型アサート
+		if !ok || id.Obj == nil { // 識別子でない、またはオブジェクトがnilなら早期リターン
+			return true
+		}
+		// keyまたはvalがnilでないことを確認してからObjを比較
+		if key != nil && id.Obj == key.Obj || val != nil && id.Obj == val.Obj {
+			f.Warn(id.Pos(), "range variable", id.Name, "enclosed by function")
 		}
 		return true
 	})
@@ -101,4 +105,13 @@ func BadRangeLoopsUsedInTests() {
 		println(i, v)
 	}()
 }
+	// If the key of the range statement is not an identifier
+	// the code should not panic (it used to).
+	var x [2]int
+	var f int
+	for x[0], f = range s { // キーが識別子ではないケースのテスト
+		go func() {
+			_ = f // ERROR "range variable f enclosed by function"
+		}()
+	}
 }

コアとなるコードの解説

変更されたコードブロックは、range ループのボディ内を走査し、クロージャによってキャプチャされる range 変数を特定するロジックです。

変更前:

if n, ok := n.(*ast.Ident); ok && n.Obj != nil && (n.Obj == key.Obj || n.Obj == val.Obj) {
    f.Warn(n.Pos(), "range variable", n.Name, "enclosed by function")
}

この行は、走査中のASTノード n が *ast.Ident であり、その識別子が参照するオブジェクト (n.Obj) が nil でなく、さらにそのオブジェクトが range ループの key または val 変数に対応するオブジェクトである場合に警告を発していました。 しかし、前述の通り、key が *ast.Ident ではない場合、key 変数自体が nil になり、key.Obj にアクセスしようとするとパニックが発生しました。

変更後:

id, ok := n.(*ast.Ident)
if !ok || id.Obj == nil {
    return true
}
if key != nil && id.Obj == key.Obj || val != nil && id.Obj == val.Obj {
    f.Warn(id.Pos(), "range variable", id.Name, "enclosed by function")
}

1. id, ok := n.(*ast.Ident): まず、現在のASTノード n を *ast.Ident 型に型アサートします。ok はアサートが成功したかどうかを示します。
2. if !ok || id.Obj == nil { return true }:
   • !ok: n が *ast.Ident ではない場合（例: リテラル、式など）。
   • id.Obj == nil: n が *ast.Ident であっても、それが参照するオブジェクト (Obj) が nil の場合（これは通常、未解決の識別子や特殊なケースで発生します）。
   • これらの条件のいずれかが真であれば、現在のノードは range 変数として適切にチェックできないため、それ以上処理せずに true を返して次のノードの走査に進みます。これにより、無効な状態でのアクセスを防ぎます。
3. if key != nil && id.Obj == key.Obj || val != nil && id.Obj == val.Obj { ... }:
   • この行がパニック修正の核心です。key != nil と val != nil のチェックが追加されました。
   • これにより、key または val が nil である（つまり、range ループのキーや値が *ast.Ident ではない、または存在しない）場合に、nil ポインタデリファレンスを伴う Obj へのアクセスを試みることを防ぎます。
   • id.Obj == key.Obj または id.Obj == val.Obj の比較は、走査中の識別子 id が、range ループの key または val 変数と実際に同じオブジェクトを参照しているかどうかを確認します。

この修正により、go vet は range ループのキーが配列のインデックスや構造体のフィールドなど、*ast.Ident ではない複雑な式である場合でも、安全かつ正確に rangeloop の警告を検出できるようになりました。

また、コミットの最後に追加されたテストコードは、この修正が意図通りに機能することを確認しています。

	// If the key of the range statement is not an identifier
	// the code should not panic (it used to).
	var x [2]int
	var f int
	for x[0], f = range s { // キーが識別子ではない例
		go func() {
			_ = f // ERROR "range variable f enclosed by function"
		}()
	}

このテストケースは、for x[0], f = range s のようにキーが x[0]（配列の要素）である場合に、go vet がパニックを起こさずに f がクロージャに囲まれていることを正しく警告することを示しています。

関連リンク

• Go CL (Change List): https://golang.org/cl/6540045
• GitHub Commit: https://github.com/golang/go/commit/5a93fea08e7c9cbc4ed5ab7ba161b3e078497fb3

参考にした情報源リンク

• Go言語の for ステートメント (The Go Programming Language Specification): https://go.dev/ref/spec#For_statements
• go vet コマンド (Go Command Documentation): https://go.dev/cmd/vet/
• go/ast パッケージ (Go Package Documentation): https://pkg.go.dev/go/ast
• Go言語におけるクロージャとループ変数のキャプチャに関する一般的な情報源 (例: Effective Go - Loop variables in closures): https://go.dev/doc/effective_go#closures