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

このコミットは、Go言語のコードフォーマッタであるgofmtが使用するgo/printerパッケージにおけるバグ修正と、セレクタ式の処理の簡素化を目的としています。具体的には、セレクタ式を再書き込みする際に、意味的に重要な括弧が失われる問題を解決し、同時にコードの可読性と保守性を向上させています。

コミット

commit 47afa4dba53c0528b7a9b06a44dd14529ad955d6
Author: Robert Griesemer <gri@golang.org>
Date:   Wed Feb 15 12:25:37 2012 -0800

    go/printer: don't lose relevant parentheses when rewriting selector expressions
    
    Also: Simplified handling of selector expressions. As a result, complicated
    multi-line expressions containing selectors and calls/indices with arguments
    broken accross lines don't get indented the same way as before, but the change
    is minimal (see tests) and there's no such code in the std library. It seems
    a worthwhile compromise given the much simpler code.
    
    Applied gofmt -w $GOROOT/src $GOROOT/misc .
    
    Fixes #1847.
    
    R=rsc
    CC=golang-dev
    https://golang.org/cl/5675062

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

https://github.com/golang/go/commit/47afa4dba53c0528b7a9b06a44dd14529ad955d6

元コミット内容

このコミットの主な目的は、go/printerパッケージがセレクタ式（例: obj.fieldやobj.method()）を整形する際に、本来保持されるべき括弧を誤って削除してしまうバグを修正することです。また、セレクタ式の処理ロジックを簡素化し、コードベース全体のgofmt適用も行われています。

変更の背景

Go言語のコードフォーマッタであるgofmtは、Goコードの構文木（AST: Abstract Syntax Tree）を解析し、標準的なスタイルに整形して出力します。この整形処理において、特定の種類の式、特にセレクタ式と括弧が組み合わされた場合に、go/printerが式の意味を変えてしまうような不適切な整形を行うバグが存在していました。

具体的には、(*x).fのような式において、go/printerが括弧を削除して*x.fと整形してしまう問題がありました。これは、ポインタのデリファレンスとフィールドアクセス（セレクタ）の結合順序が変更され、プログラムの意味が変わってしまう深刻なバグです。(*x).fはxが指す構造体のfフィールドにアクセスしますが、*x.fはx.f（xのfフィールド）をデリファレンスしようとします。もしxがポインタ型で、fがポインタでない場合、これはコンパイルエラーになるか、全く異なる動作を引き起こす可能性があります。

この問題は、Goの内部イシュートラッカーで#1847として報告されていました。このコミットは、この重要なバグを修正し、go/printerが式の意味論を損なうことなく整形できるようにすることを目的としています。

前提知識の解説

Go言語の構文木 (AST)

Goコンパイラやツール（gofmtなど）は、Goのソースコードを直接扱うのではなく、そのコードの抽象的な構造を表現する構文木（AST）に変換して処理します。ASTは、プログラムの各要素（変数、関数、式、文など）をノードとして表現し、それらの関係をツリー構造で表します。go/astパッケージがASTの定義を提供し、go/parserがソースコードをASTに変換し、go/printerがASTをソースコードに変換（整形して出力）します。

セレクタ式 (Selector Expressions)

Go言語において、セレクタ式はドット（.）を使用して、構造体のフィールドやメソッド、あるいはパッケージの公開された識別子にアクセスするために使用されます。例:

obj.Field (構造体objのFieldフィールド)
obj.Method() (構造体objのMethodメソッドの呼び出し)
pkg.Constant (パッケージpkgのConstant)

演算子の優先順位と結合規則

プログラミング言語では、複数の演算子が組み合わされた式において、どの演算子が先に評価されるかを決定するルールがあります。これが「演算子の優先順位」です。また、同じ優先順位の演算子が並んだ場合に、左から右、または右から左のどちらに結合するかを決定するルールが「結合規則」です。

Go言語における主要な演算子の優先順位（高い順）:

単項演算子（+, -, !, ^, * (デリファレンス), & (アドレス取得), <- (チャネル受信)）
乗算、除算、剰余、ビットAND、ビットOR、ビットXOR、左シフト、右シフト
加算、減算、ビットOR、ビットXOR
比較演算子
論理AND
論理OR

セレクタ（.）は、単項演算子よりも高い優先順位を持ちます。このため、*x.fという式は、x.fが先に評価され、その結果がデリファレンスされることを意味します。一方、(*x).fという式は、括弧によって*xが先に評価され、その結果（xが指す値）のfフィールドにアクセスすることを意味します。この違いが、今回のバグの根本原因でした。

`go/printer`パッケージ

go/printerパッケージは、GoのASTを整形されたGoソースコードに変換する役割を担っています。これはgofmtツールの中心的なコンポーネントです。go/printerは、ASTノードをトラバースし、Goの標準的なフォーマット規則に従って空白、改行、インデントなどを挿入しながらコードを生成します。このパッケージの正確性は、Goコードの可読性と一貫性を保つ上で極めて重要です。

技術的詳細

このコミットの技術的な核心は、src/pkg/go/printer/nodes.goファイルにおけるprinter構造体のexpr1メソッドの変更にあります。このメソッドは、GoのASTにおける単一の式ノードを整形して出力する役割を担っています。

以前の実装では、*ast.SelectorExpr（セレクタ式）を処理する際に、selectorExprListというヘルパー関数を使用してセレクタチェーンを分解し、exprListメソッドで整形していました。このselectorExprList関数は、セレクタチェーンをドットで区切られた部分に分割するものでしたが、この過程で括弧の情報を適切に保持できないケースがありました。特に、(*x).fのように、セレクタの左側が括弧で囲まれた式である場合に、その括弧が「意味的に重要」であるにもかかわらず失われてしまう問題がありました。

新しい実装では、selectorExprList関数とperiodSepモードが削除され、セレクタ式の処理が大幅に簡素化されました。 *ast.SelectorExprの処理は以下のように変更されました。

まず、セレクタの左側の式 (x.X) を再帰的にp.expr1で整形します。この際、token.HighestPrec（最高優先順位）を指定することで、左側の式が適切に評価されるようにします。
次に、ドット (.) を出力します。
その後、セレクタの右側の識別子 (x.Sel) を出力します。ここで、もしセレクタの右側が新しい行に移動している場合（マルチラインの場合）、適切なインデントと改行を挿入し、*multiLineフラグをtrueに設定します。

この変更により、go/printerはセレクタ式をより直接的に処理するようになり、(*x).fのような式で括弧が失われることがなくなりました。なぜなら、x.Xが(*x)として整形される際に、その括弧がexpr1によって適切に保持されるためです。

また、コミットメッセージにあるように、この変更により、複雑な複数行にわたるセレクタ式や、引数が複数行にわたる関数呼び出し/インデックス式を含むセレクタ式において、以前とは異なるインデントが適用される可能性があります。しかし、これは標準ライブラリにはそのようなコードが存在しないこと、およびコードの簡素化というメリットを考慮すると、許容できる妥協点とされています。

misc/dashboard/builder/main.goの変更は、GOBUILDEXIT環境変数の設定におけるアライメントの修正であり、本質的な機能変更ではありません。 src/cmd/gofmt/gofmt_test.goとsrc/cmd/gofmt/testdata/rewrite4.input/rewrite4.goldenの追加は、このバグ修正を検証するための新しいテストケースです。特にrewrite4.inputには、(-x).f、(*x).f、(&x).f、(!x).fといった、括弧が意味的に重要なセレクタ式のパターンが含まれており、これらが正しく整形されることを確認しています。

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

主要な変更はsrc/pkg/go/printer/nodes.goファイルに集中しています。

`src/pkg/go/printer/nodes.go`

--- a/src/pkg/go/printer/nodes.go
+++ b/src/pkg/go/printer/nodes.go
@@ -87,7 +87,6 @@ const (
 	commaSep                            // elements are separated by commas
 	commaTerm                           // list is optionally terminated by a comma
 	noIndent                            // no extra indentation in multi-line lists
-	periodSep                           // elements are separated by periods
 )
 
 // Sets multiLine to true if the identifier list spans multiple lines.
@@ -213,13 +212,10 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
 		}
 
 		if i > 0 {
-\t\t\tswitch {\n-\t\t\tcase mode&commaSep != 0:\n+\t\t\tif mode&commaSep != 0 {\n \t\t\t\tp.print(token.COMMA)\n-\t\t\tcase mode&periodSep != 0:\n-\t\t\t\tp.print(token.PERIOD)\n \t\t\t}\n-\t\t\tneedsBlank := mode&periodSep == 0 // period-separated list elements don\'t need a blank
+\t\t\tneedsBlank := true
 \t\t\tif prevLine < line && prevLine > 0 && line > 0 {
 \t\t\t\t// lines are broken using newlines so comments remain aligned
 \t\t\t\t// unless forceFF is set or there are multiple expressions on
@@ -668,63 +664,6 @@ func isBinary(expr ast.Expr) bool {
 	return ok
 }
 
-// If the expression contains one or more selector expressions, splits it into
-// two expressions at the rightmost period. Writes entire expr to suffix when
-// selector isn\'t found. Rewrites AST nodes for calls, index expressions and
-// type assertions, all of which may be found in selector chains, to make them
-// parts of the chain.\n-func splitSelector(expr ast.Expr) (body, suffix ast.Expr) {
-\t\tswitch x := expr.(type) {
-\t\tcase *ast.SelectorExpr:\n-\t\t\tbody, suffix = x.X, x.Sel
-\t\t\treturn
-\t\tcase *ast.CallExpr:\n-\t\t\tbody, suffix = splitSelector(x.Fun)
-\t\t\tif body != nil {
-\t\t\t\tsuffix = &ast.CallExpr{suffix, x.Lparen, x.Args, x.Ellipsis, x.Rparen}
-\t\t\t\treturn
-\t\t\t}
-\t\tcase *ast.IndexExpr:\n-\t\t\tbody, suffix = splitSelector(x.X)
-\t\t\tif body != nil {
-\t\t\t\tsuffix = &ast.IndexExpr{suffix, x.Lbrack, x.Index, x.Rbrack}
-\t\t\t\treturn
-\t\t\t}
-\t\tcase *ast.SliceExpr:\n-\t\t\tbody, suffix = splitSelector(x.X)
-\t\t\tif body != nil {
-\t\t\t\tsuffix = &ast.SliceExpr{suffix, x.Lbrack, x.Low, x.High, x.Rbrack}
-\t\t\t\treturn
-\t\t\t}
-\t\tcase *ast.TypeAssertExpr:\n-\t\t\tbody, suffix = splitSelector(x.X)
-\t\t\tif body != nil {
-\t\t\t\tsuffix = &ast.TypeAssertExpr{suffix, x.Type}
-\t\t\t\treturn
-\t\t\t}
-\t\t}
-\t\tsuffix = expr
-\t\treturn
-\t}
-\n-// Convert an expression into an expression list split at the periods of
-// selector expressions.\n-func selectorExprList(expr ast.Expr) (list []ast.Expr) {
-\t// split expression
-\tfor expr != nil {
-\t\tvar suffix ast.Expr
-\t\texpr, suffix = splitSelector(expr)
-\t\tlist = append(list, suffix)
-\t}
-\n-\t// reverse list
-\tfor i, j := 0, len(list)-1; i < j; i, j = i+1, j-1 {
-\t\tlist[i], list[j] = list[j], list[i]
-\t}
-\n-\treturn
-}
-\n // Sets multiLine to true if the expression spans multiple lines.
 func (p *printer) expr1(expr ast.Expr, prec1, depth int, multiLine *bool) {
 \tp.print(expr.Pos())\n@@ -798,8 +737,14 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int, multiLine *bool) {
 \t\t}\n \n \tcase *ast.SelectorExpr:\n-\t\tparts := selectorExprList(expr)\n-\t\tp.exprList(token.NoPos, parts, depth, periodSep, multiLine, token.NoPos)\n+\t\tp.expr1(x.X, token.HighestPrec, depth, multiLine)\n+\t\tp.print(token.PERIOD)\n+\t\tif line := p.lineFor(x.Sel.Pos()); p.pos.IsValid() && p.pos.Line < line {\n+\t\t\tp.print(indent, newline, x.Sel.Pos(), x.Sel, unindent)\n+\t\t\t*multiLine = true\n+\t\t} else {\n+\t\t\tp.print(x.Sel.Pos(), x.Sel)\n+\t\t}\n \n \tcase *ast.TypeAssertExpr:\n \t\tp.expr1(x.X, token.HighestPrec, depth, multiLine)\n```

## コアとなるコードの解説

### `periodSep`の削除

`src/pkg/go/printer/nodes.go`の定数定義から`periodSep`が削除されました。これは、セレクタ式をドットで区切られたリストとして扱う古いアプローチが廃止されたことを示しています。

```go
 const (
 	commaSep                            // elements are separated by commas
 	commaTerm                           // list is optionally terminated by a comma
 	noIndent                            // no extra indentation in multi-line lists
-	periodSep                           // elements are separated by periods
 )

`exprList`メソッドの変更

exprListメソッド内のswitch文が簡素化され、periodSepに関する処理が削除されました。これにより、リスト要素間の空白の扱いも簡素化されています。

--- a/src/pkg/go/printer/nodes.go
+++ b/src/pkg/go/printer/nodes.go
@@ -213,13 +212,10 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
 		}
 
 		if i > 0 {
-\t\t\tswitch {\n-\t\t\tcase mode&commaSep != 0:\n+\t\t\tif mode&commaSep != 0 {\n \t\t\t\tp.print(token.COMMA)\n-\t\t\tcase mode&periodSep != 0:\n-\t\t\t\tp.print(token.PERIOD)\n \t\t\t}\n-\t\t\tneedsBlank := mode&periodSep == 0 // period-separated list elements don\'t need a blank
+\t\t\tneedsBlank := true
 \t\t\tif prevLine < line && prevLine > 0 && line > 0 {
 \t\t\t\t// lines are broken using newlines so comments remain aligned
 \t\t\t\t// unless forceFF is set or there are multiple expressions on

`splitSelector`および`selectorExprList`関数の削除

セレクタ式を分解してリスト化するsplitSelectorおよびselectorExprList関数が完全に削除されました。これは、セレクタ式の処理ロジックが根本的に変更され、これらのヘルパー関数が不要になったことを意味します。

--- a/src/pkg/go/printer/nodes.go
+++ b/src/pkg/go/printer/nodes.go
@@ -668,63 +664,6 @@ func isBinary(expr ast.Expr) bool {
 	return ok
 }
 
-// If the expression contains one or more selector expressions, splits it into
-// two expressions at the rightmost period. Writes entire expr to suffix when
-// selector isn\'t found. Rewrites AST nodes for calls, index expressions and
-// type assertions, all of which may be found in selector chains, to make them
-// parts of the chain.\n-func splitSelector(expr ast.Expr) (body, suffix ast.Expr) {
-\t\tswitch x := expr.(type) {
-\t\tcase *ast.SelectorExpr:\n-\t\t\tbody, suffix = x.X, x.Sel
-\t\t\treturn
-\t\tcase *ast.CallExpr:\n-\t\t\tbody, suffix = splitSelector(x.Fun)
-\t\t\tif body != nil {
-\t\t\t\tsuffix = &ast.CallExpr{suffix, x.Lparen, x.Args, x.Ellipsis, x.Rparen}
-\t\t\t\treturn
-\t\t\t}
-\t\tcase *ast.IndexExpr:\n-\t\t\tbody, suffix = splitSelector(x.X)
-\t\t\tif body != nil {
-\t\t\t\tsuffix = &ast.IndexExpr{suffix, x.Lbrack, x.Index, x.Rbrack}
-\t\t\t\treturn
-\t\t\t}
-\t\tcase *ast.SliceExpr:\n-\t\t\tbody, suffix = splitSelector(x.X)
-\t\t\tif body != nil {
-\t\t\t\tsuffix = &ast.SliceExpr{suffix, x.Lbrack, x.Low, x.High, x.Rbrack}
-\t\t\t\treturn
-\t\t\t}
-\t\tcase *ast.TypeAssertExpr:\n-\t\t\tbody, suffix = splitSelector(x.X)
-\t\t\tif body != nil {
-\t\t\t\tsuffix = &ast.TypeAssertExpr{suffix, x.Type}
-\t\t\t\treturn
-\t\t\t}
-\t\t}
-\t\tsuffix = expr
-\t\treturn
-\t}
-\n-// Convert an expression into an expression list split at the periods of
-// selector expressions.\n-func selectorExprList(expr ast.Expr) (list []ast.Expr) {
-\t// split expression
-\tfor expr != nil {
-\t\tvar suffix ast.Expr
-\t\texpr, suffix = splitSelector(expr)
-\t\tlist = append(list, suffix)
-\t}
-\n-\t// reverse list
-\tfor i, j := 0, len(list)-1; i < j; i, j = i+1, j-1 {
-\t\tlist[i], list[j] = list[j], list[i]
-\t}
-\n-\treturn
-}
-\n // Sets multiLine to true if the expression spans multiple lines.
 func (p *printer) expr1(expr ast.Expr, prec1, depth int, multiLine *bool) {
  p.print(expr.Pos())

`expr1`メソッド内の`*ast.SelectorExpr`の処理変更

これが最も重要な変更点です。セレクタ式を処理するロジックが、より直接的で再帰的なアプローチに変更されました。

--- a/src/pkg/go/printer/nodes.go
+++ b/src/pkg/go/printer/nodes.go
@@ -798,8 +737,14 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int, multiLine *bool) {
 		}\n \n \tcase *ast.SelectorExpr:\n-\t\tparts := selectorExprList(expr)\n-\t\tp.exprList(token.NoPos, parts, depth, periodSep, multiLine, token.NoPos)\n+\t\tp.expr1(x.X, token.HighestPrec, depth, multiLine)\n+\t\tp.print(token.PERIOD)\n+\t\tif line := p.lineFor(x.Sel.Pos()); p.pos.IsValid() && p.pos.Line < line {\n+\t\t\tp.print(indent, newline, x.Sel.Pos(), x.Sel, unindent)\n+\t\t\t*multiLine = true\n+\t\t} else {\n+\t\t\tp.print(x.Sel.Pos(), x.Sel)\n+\t\t}\n \n \tcase *ast.TypeAssertExpr:\
 \t\tp.expr1(x.X, token.HighestPrec, depth, multiLine)\

変更前:

	case *ast.SelectorExpr:
		parts := selectorExprList(expr)
		p.exprList(token.NoPos, parts, depth, periodSep, multiLine, token.NoPos)

変更後:

	case *ast.SelectorExpr:
		p.expr1(x.X, token.HighestPrec, depth, multiLine) // セレクタの左側を再帰的に整形
		p.print(token.PERIOD)                             // ドットを出力
		if line := p.lineFor(x.Sel.Pos()); p.pos.IsValid() && p.pos.Line < line {
			// セレクタの右側が改行されている場合、インデントと改行を挿入
			p.print(indent, newline, x.Sel.Pos(), x.Sel, unindent)
			*multiLine = true
		} else {
			// そうでない場合、そのまま出力
			p.print(x.Sel.Pos(), x.Sel)
		}

この新しいロジックでは、セレクタ式x.Yを整形する際に、まずxの部分を再帰的にexpr1で整形します。この再帰呼び出しが、(*expr)のような括弧で囲まれた式を適切に処理し、その括弧を保持します。その後、ドットとYの部分を整形します。これにより、以前のようにセレクタチェーンを分解する際に括弧が失われる問題が解決されました。

参考にした情報源リンク

コミットメッセージ自体
Go言語の公式ドキュメント（特にgo/ast, go/parser, go/printerパッケージに関するもの）
Go言語の演算子の優先順位に関する一般的な情報
src/cmd/gofmt/testdata/rewrite4.input および rewrite4.golden ファイルの内容
src/pkg/go/printer/nodes.go の変更前後のコード
Go言語のIssue Tracker (ただし、#1847は公開されている情報が少ないため、コミットメッセージからの推測が主)

comemo