Keyboard shortcuts

Press or to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

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

このコミットは、net/http パッケージ内の client_test.go ファイルにおける、定義されたが使用されていない変数の削除に関するものです。具体的には、TestClientTimeout 関数内で宣言されていた all []byte 変数が、クロージャ内で ioutil.ReadAll の結果を受け取るために使用されていましたが、その値が後続の処理で利用されていなかったため、削除されました。

コミット

commit cf57cf17e785e0e1c36c522067490b9a806e9cb1
Author: Alan Donovan <adonovan@google.com>
Date:   Mon Mar 10 22:22:51 2014 -0400

    net/http: eliminate defined-but-not-used var.
    
    gc does not report this as an error, but go/types does.
    (I suspect that constructing a closure counts as a reference
    to &all in gc's implementation).
    
    This is not a tool bug, since the spec doesn't require
    implementations to implement this check, but it does
    illustrate that dialect variations are always a nuisance.
    
    LGTM=rsc, bradfitz
    R=bradfitz
    CC=golang-codereviews, gri, rsc
    https://golang.org/cl/73850043

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

https://github.com/golang/go/commit/cf57cf17e785e0e1c36c522067490b9a806e9cb1

元コミット内容

net/http: eliminate defined-but-not-used var.

gc does not report this as an error, but go/types does.
(I suspect that constructing a closure counts as a reference
to &all in gc's implementation).

This is not a tool bug, since the spec doesn't require
implementations to implement this check, but it does
illustrate that dialect variations are always a nuisance.

変更の背景

この変更の背景には、Go言語のコンパイラ(gc)と型チェッカー(go/types)の間での「定義されたが使用されていない変数」の扱いに関する挙動の違いがあります。

Go言語では、宣言された変数がコード内で一度も使用されない場合、通常はコンパイルエラーまたは警告となります。これは、デッドコードの排除や、プログラマが意図しない変数を宣言してしまった場合のバグの早期発見に役立つため、良いプラクティスとされています。

しかし、このコミットがなされた時点では、Goの公式コンパイラであるgcは、特定の状況下(特にクロージャ内で変数が参照される場合)において、変数が実際に使用されていなくてもエラーとして報告しないことがありました。一方で、go/typesパッケージ(これはGoのソースコードを解析し、型情報を抽出するためのライブラリであり、IDEやリンターなどのツールで利用されます)は、より厳密にこのルールを適用し、エラーとして検出していました。

コミットメッセージにある「dialect variations are always a nuisance」という表現は、異なるツールや実装間で同じコードに対する解釈が異なることが、開発者にとって混乱や不便をもたらすという問題意識を示しています。このコミットは、このようなツールの挙動の不一致を解消し、コードベースの一貫性と品質を向上させることを目的としています。具体的には、go/typesがエラーとして検出するようなケースを修正することで、より厳密なコードチェックに適合させようとしています。

前提知識の解説

Go言語の変数宣言と使用

Go言語では、変数を宣言する際にvarキーワードを使用します。例えば、var x intのように宣言します。宣言された変数がそのスコープ内で一度も読み取られたり、書き込まれたりしない場合、Goコンパイラは通常、コンパイルエラーを発生させます。これは、Goが「宣言されたが使用されていない変数」をエラーとして扱うためです。この厳格なチェックは、意図しないバグやデッドコードを防ぐのに役立ちます。

Goコンパイラ (gc)

gcは、Go言語の公式コンパイラであり、Goのソースコードを機械語に変換する役割を担っています。gcは、Goプログラムのビルドプロセスの中核をなすツールです。変数の使用状況のチェックもgcの機能の一部ですが、このコミットが示唆するように、特定のケース(特にクロージャとの関連)では、そのチェックがgo/typesほど厳密ではないことがありました。

go/typesパッケージ

go/typesは、Go言語の標準ライブラリの一部であり、Goのソースコードの型チェックを行うためのパッケージです。これは、コンパイラが内部的に使用する型システムを公開しており、Goのコードを静的に解析するツール(例えば、IDE、リンター、コード分析ツールなど)が、プログラムのセマンティックな正確性を検証するために利用します。go/typesは、Goの言語仕様に厳密に従って型チェックを行うため、gcよりも厳格なチェックを行うことがあります。

クロージャ (Closures)

Go言語におけるクロージャは、関数がその関数が定義された環境(レキシカルスコープ)の変数を「記憶」し、その変数にアクセスできる機能を持つ関数です。クロージャは、匿名関数として定義され、その関数が定義された時点の外部スコープの変数をキャプチャします。

このコミットの文脈では、all []byte変数がクロージャ内で参照されていました。gcがこの変数を「使用されている」と判断した可能性は、クロージャがall変数をキャプチャしたこと自体を、gcが変数への参照と見なしたためかもしれません。しかし、go/typesは、変数の値が実際に読み取られて利用される場合にのみ「使用されている」と判断するため、この違いが生じました。

ioutil.ReadAll

ioutil.ReadAllは、Goの標準ライブラリio/ioutilパッケージ(Go 1.16以降はioパッケージに移行)にある関数で、io.Readerインターフェースを実装するソースからすべてのデータを読み込み、[]byteスライスとして返します。この関数は、ファイルの内容を読み込んだり、HTTPレスポンスボディを読み込んだりする際によく使用されます。

技術的詳細

このコミットの技術的な核心は、Go言語のコンパイラ(gc)と静的解析ツール(go/types)の間で、「定義されたが使用されていない変数」の検出ロジックに差異があった点にあります。

元のコードでは、TestClientTimeout関数内でall []byteという変数が宣言され、その直後に起動されるゴルーチン内のクロージャでioutil.ReadAll(res.Body)の結果を受け取るために使用されていました。

// 変更前
var all []byte
errc := make(chan error, 1)
go func() {
    var err error
    all, err = ioutil.ReadAll(res.Body) // ここで 'all' に値が代入される
    errc <- err
    res.Body.Close()
}()

ここで重要なのは、all変数にioutil.ReadAllの結果が代入されているものの、そのall変数の値がクロージャの外でも内でも、その後のコードで一切使用されていないという点です。

コミットメッセージによると、gcはこのall変数を「使用されている」と判断し、エラーを報告しませんでした。これは、gcがクロージャが外部変数(この場合はall)をキャプチャする行為自体を、変数への「参照」と見なしたためであると推測されています。Goのクロージャは、外部スコープの変数を参照する際に、その変数のアドレスをキャプチャすることがあります。gcは、このアドレスのキャプチャを、変数が「使用されている」と判断する十分な理由と見なした可能性があります。

一方、go/typesは、より厳密なセマンティック分析を行います。go/typesは、変数が実際にその値が読み取られたり、何らかの計算に使用されたりする場合にのみ「使用されている」と判断します。単に値が代入されたり、クロージャによって参照されたりするだけでは、「使用されている」とは見なさないのです。このため、go/typesall変数が定義されたが使用されていないと判断し、エラーを報告しました。

この挙動の不一致は、開発者がgcでコンパイルが通るにもかかわらず、go/typesを使用するリンターやIDEで警告やエラーが表示されるという状況を生み出します。これは、開発ワークフローにおいて「方言の不一致 (dialect variations)」として認識され、不便さの原因となります。

このコミットは、この不一致を解消するために、all変数が実際に使用されていないのであれば、その代入結果を破棄するように変更しました。

// 変更後
_, err := ioutil.ReadAll(res.Body) // 'all' 変数を削除し、結果を破棄

_(ブランク識別子)を使用することで、ioutil.ReadAllが返す[]byteスライスを明示的に破棄し、その値が不要であることをコンパイラに伝えます。これにより、all変数を宣言する必要がなくなり、go/typesが報告していた「定義されたが使用されていない変数」のエラーが解消されます。

この変更は、Go言語の仕様自体が「定義されたが使用されていない変数」のチェックを実装に義務付けているわけではない、というコミットメッセージの補足も重要です。しかし、実用的な観点から、ツール間の一貫性を保ち、開発者の混乱を避けるために、このような修正が行われました。これは、Goエコシステム全体の品質と開発体験を向上させるための継続的な取り組みの一環と言えます。

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

変更は src/pkg/net/http/client_test.go ファイルの TestClientTimeout 関数内で行われました。

--- a/src/pkg/net/http/client_test.go
+++ b/src/pkg/net/http/client_test.go
@@ -886,11 +886,9 @@ func TestClientTimeout(t *testing.T) {
 		t.Fatal("handler never got /slow request")
 	}
 
-	var all []byte
 	errc := make(chan error, 1)
 	go func() {
-		var err error
-		all, err = ioutil.ReadAll(res.Body)
+		_, err := ioutil.ReadAll(res.Body)
 		errc <- err
 		res.Body.Close()
 	}()

具体的には、以下の変更が行われました。

  1. var all []byte の行が削除されました。
  2. クロージャ内の all, err = ioutil.ReadAll(res.Body) の行が _, err := ioutil.ReadAll(res.Body) に変更されました。

コアとなるコードの解説

この変更は、TestClientTimeout 関数内のゴルーチンで実行されるクロージャの内部ロジックを簡素化し、不要な変数を排除することを目的としています。

変更前は、以下のようになっていました。

var all []byte // (1) 'all' 変数を宣言
errc := make(chan error, 1)
go func() {
    var err error
    all, err = ioutil.ReadAll(res.Body) // (2) 'all' にレスポンスボディを読み込んだ結果を代入
    errc <- err
    res.Body.Close()
}()
  1. all []byte は、HTTPレスポンスボディを読み込むためのバイトスライスとして宣言されていました。
  2. ゴルーチン内で実行されるクロージャの中で、ioutil.ReadAll(res.Body) の結果が all 変数に代入されていました。しかし、この all 変数の値は、このクロージャ内でも、またクロージャが実行されている外部の TestClientTimeout 関数内でも、その後の処理で一切使用されていませんでした。つまり、レスポンスボディを読み込むこと自体は必要でしたが、その内容を保持する必要はなかったのです。

変更後は、以下のようになります。

errc := make(chan error, 1)
go func() {
    _, err := ioutil.ReadAll(res.Body) // (1) 'all' 変数を削除し、結果をブランク識別子で破棄
    errc <- err
    res.Body.Close()
}()
  1. all []byte の宣言が削除されました。
  2. all, err = ioutil.ReadAll(res.Body) の代わりに _, err := ioutil.ReadAll(res.Body) が使用されています。ここで使用されている _ (ブランク識別子) は、Go言語において、関数が返す値のうち、特定の値を明示的に破棄したい場合に使用されます。ioutil.ReadAll は2つの値を返します(読み込んだバイトスライスとエラー)。この変更では、読み込んだバイトスライス(allに代入されていた部分)は不要であるため、_ を使ってその値を破棄しています。これにより、all変数を宣言する必要がなくなり、「定義されたが使用されていない変数」の問題が解消されます。

この修正により、コードはよりクリーンになり、go/typesのような静的解析ツールが報告する警告やエラーがなくなります。これは、コードの品質を向上させ、開発者がより一貫したツール体験を得るのに役立ちます。

関連リンク

参考にした情報源リンク

  • Go言語の公式ドキュメント
  • Go言語のソースコード
  • Go言語のコミュニティフォーラムやメーリングリスト(一般的なGoのプラクティスやツールの挙動に関する議論)
  • Go言語のクロージャに関する一般的な解説記事
  • Go言語のブランク識別子に関する解説記事