[インデックス 11841] ファイルの概要
このコミットは、Goコンパイラ(gc
)、アセンブラ(8l
)、および8gコンパイラ(8g
)における複数の警告を修正するものです。具体的には、print
関数のフォーマット指定子の不一致、未使用変数の削除、誤解を招く比較の修正、関数のリンケージ指定の修正、未使用引数の削除、到達不能なコードの削除など、様々なコード品質と正確性に関する改善が含まれています。これらの修正は、コンパイラの堅牢性と保守性を向上させることを目的としています。
コミット
- コミットハッシュ:
dbec42104f5a7b177ed04098ca4cfb7a1659b9b1
- Author:
Anthony Martin <ality@pbrane.org>
- Date:
Sun Feb 12 23:07:31 2012 -0800
GitHub上でのコミットページへのリンク
https://github.com/golang/go/commit/dbec42104f5a7b177ed04098ca4cfb7a1659b9b1
元コミット内容
gc, 8g, 8l: fix a handful of warnings
8g/cgen.c
print format type mismatch
8l/asm.c
resoff set and not used
gc/pgen.c
misleading comparison INT > 0x80000000
gc/reflect.c
dalgsym must be static to match forward declaration
gc/subr.c
assumed_equal set and not used
hashmem's second argument is not used
gc/walk.c
duplicated (unreachable) code
R=rsc
CC=golang-dev
https://golang.org/cl/5651079
変更の背景
このコミットは、Goコンパイラおよび関連ツールチェーンにおける既存の警告を解消し、コードベースの健全性を向上させることを目的としています。警告は、潜在的なバグ、非効率なコード、または誤解を招く可能性のあるコードパターンを示すことがよくあります。これらの警告を修正することで、コンパイラの信頼性が高まり、将来の開発者がコードを理解しやすくなります。
具体的には、以下のような問題が修正されています。
- 型ミスマッチ:
print
関数のフォーマット指定子と引数の型が一致しない場合、予期せぬ出力やクラッシュにつながる可能性があります。 - 未使用変数/引数: 未使用の変数や引数は、コードの可読性を低下させ、誤解を招く可能性があります。また、コンパイラが最適化を行う際に余分な作業を必要とする場合があります。
- 誤解を招く比較: 符号付き整数と符号なし整数、または異なるサイズの整数を比較する際に、意図しない結果を招く可能性があります。
- リンケージの問題: C言語において、関数の前方宣言と定義のリンケージ(
static
など)が一致しない場合、コンパイルエラーやリンクエラーの原因となります。 - 到達不能なコード: 実行されることのないコードは、デッドコードと呼ばれ、コードベースを不必要に複雑にし、保守を困難にします。
これらの問題に対処することで、Goコンパイラの品質と保守性が向上します。
前提知識の解説
このコミットを理解するためには、以下の概念に関する基本的な知識が必要です。
- Goコンパイラツールチェーン:
gc
: Go言語のコンパイラ本体。Goのソースコードを中間表現に変換し、最終的にアセンブリコードを生成します。8g
:gc
の一部で、x86アーキテクチャ(32ビット)向けのGoコンパイラフロントエンドを指します。Goのソースコードを解析し、中間コードを生成します。8l
:gc
の一部で、x86アーキテクチャ(32ビット)向けのGoリンカを指します。コンパイルされたオブジェクトファイルを結合し、実行可能ファイルを生成します。cmd
: Goのソースツリーにおけるコマンド(ツール)のディレクトリ。コンパイラやリンカなどのツールが含まれます。
- C言語: Goコンパイラの初期バージョンはC言語で書かれており、このコミットで修正されているファイルもC言語のソースファイルです。
print
関数とフォーマット指定子: C言語の標準出力関数で、%d
(整数)、%ld
(long整数)、%lld
(long long整数)などのフォーマット指定子を使用して、様々な型のデータを整形して出力します。フォーマット指定子と引数の型が一致しないと、未定義の動作を引き起こす可能性があります。static
キーワード: C言語において、static
キーワードは変数や関数のスコープとリンケージを制御します。関数にstatic
を付けると、その関数は定義されたファイル内でのみ可視となり、他のファイルからはアクセスできなくなります(内部リンケージ)。これにより、名前の衝突を防ぎ、カプセル化を促進します。前方宣言と定義でstatic
の有無が一致しないと、コンパイルエラーやリンクエラーの原因となります。vlong
: Goコンパイラの内部で使われる、64ビット整数を表す型エイリアス(通常はlong long
に相当)。int64
: 64ビット符号付き整数型。0x80000000
: 32ビット符号付き整数の最小値(-2,147,483,648)を表現する際に、符号なし整数として解釈されると最大値に近い値(2,147,483,648)となるため、比較において誤解を招くことがあります。USED
マクロ: C言語のコードベースで、変数が意図的に使用されていないことをコンパイラに伝えるためのマクロ。通常、コンパイラの「未使用変数」警告を抑制するために使用されます。
- Goの型システムとリフレクション:
Type
: Go言語の型を表す構造体。リフレクション(実行時に型情報を検査・操作する機能)において重要な役割を果たします。Sym
: Goコンパイラの内部で、シンボル(変数名、関数名など)を表す構造体。Node
: Goコンパイラの抽象構文木(AST)におけるノードを表す構造体。hashmem
: Goのランタイムやコンパイラ内部で、メモリブロックのハッシュ値を計算するために使用される関数。eqtype1
: Goの型が等しいかどうかを比較する関数。assumed_equal
:eqtype1
関数内で、再帰的な型比較の際に無限ループを防ぐために、既に比較中であると仮定される型ペアを追跡するためのリスト。
技術的詳細
このコミットは、Goコンパイラの複数のサブシステムにわたる、様々な種類の警告を修正しています。
-
8g/cgen.c
におけるprint
フォーマット指定子の不一致:sgen
関数内のデバッグ出力で、w
というint64
型の変数を%ld
(通常はlong
型用)で出力しようとしていました。- これを
%lld
(long long
型、つまりint64
型に適したフォーマット指定子)に修正することで、正しい値が出力されるようになり、警告が解消されました。
-
8l/asm.c
におけるresoff
の未使用警告:asmb
関数内でresoff
という変数が設定されていましたが、その後使用されていませんでした。USED(resoff);
という行を追加することで、この変数が意図的に使用されていないことをコンパイラに伝え、未使用変数に関する警告を抑制しています。これは、将来的にこの変数が使用される可能性を残しつつ、現在の警告を解消するための一般的なプラクティスです。
-
gc/pgen.c
における誤解を招く比較:compile
関数内で、stksize+maxarg
というスタックフレームサイズが1ULL<<31
(2GB)を超えるかどうかをチェックしていました。1ULL<<31
は符号なしのunsigned long long
型のリテラルであり、stksize+maxarg
がvlong
(64ビット符号付き整数)であるため、比較の際に型変換のルールによって誤解を招く可能性がありました。(int64)stksize+maxarg
と明示的にint64
にキャストすることで、比較が意図した通りに符号付き64ビット整数として行われるようになり、警告が解消されました。
-
gc/reflect.c
におけるdalgsym
のリンケージ修正:dalgsym
関数が前方宣言されていましたが、その宣言と定義でstatic
キーワードの有無が一致していませんでした。dalgsym
関数をstatic
として宣言することで、その関数が定義されているファイル内でのみ可視となり、前方宣言とのリンケージが一致するようになりました。これにより、コンパイラの警告が解消され、コードの整合性が保たれます。
-
gc/subr.c
における未使用変数と未使用引数:eqtype1
関数内で、assumed_equal
変数が設定されていましたが、その後のgoto
文によって使用されないパスがありました。この変数の設定と使用されないパスを削除することで、未使用変数に関する警告が解消されました。hashmem
関数がType *t
とvlong width
の2つの引数を取っていましたが、width
引数が関数内で使用されていませんでした。hashmem
関数のシグネチャからvlong width
引数を削除し、それに合わせてhashfor
関数やgenhash
関数からのhashmem
の呼び出しも引数を1つ減らすように修正されました。これにより、未使用引数に関する警告が解消され、関数のインターフェースがより明確になりました。
-
gc/walk.c
における重複した到達不能コードの削除:walkexpr
関数内に、同じn = r; goto ret;
というコードが2回連続して記述されていました。- 最初の
n = r; goto ret;
が実行されると、その後のコードは決して実行されないため、2番目の重複したコードは到達不能でした。 - 重複した到達不能なコードを削除することで、コードベースがクリーンになり、保守性が向上しました。
これらの修正は、Goコンパイラのコードベースの品質を向上させ、潜在的な問題を未然に防ぐための重要なステップです。
コアとなるコードの変更箇所
diff --git a/src/cmd/8g/cgen.c b/src/cmd/8g/cgen.c
index 5d8be4678b..48619ac732 100644
--- a/src/cmd/8g/cgen.c
+++ b/src/cmd/8g/cgen.c
@@ -1146,7 +1146,7 @@ sgen(Node *n, Node *res, int64 w)
int32 c, q, odst, osrc;
if(debug['g']) {
- print("\nsgen w=%ld\n", w);
+ print("\nsgen w=%lld\n", w);
dump("r", n);
dump("res", res);
}
diff --git a/src/cmd/8l/asm.c b/src/cmd/8l/asm.c
index dcaa0b192b..54bda1ac84 100644
--- a/src/cmd/8l/asm.c
+++ b/src/cmd/8l/asm.c
@@ -1004,6 +1004,9 @@ asmb(void)
phsh(ph, sh);
}
+ // Additions to the reserved area must be above this line.
+ USED(resoff);
+
elfphload(&segtext);
elfphload(&segdata);
diff --git a/src/cmd/gc/pgen.c b/src/cmd/gc/pgen.c
index 8e65ba22db..f2b75d61b6 100644
--- a/src/cmd/gc/pgen.c
+++ b/src/cmd/gc/pgen.c
@@ -124,7 +124,7 @@ compile(Node *fn)
print("allocauto: %lld to %lld\n", oldstksize, (vlong)stksize);
setlineno(curfn);
- if(stksize+maxarg > (1ULL<<31))
+ if((int64)stksize+maxarg > (1ULL<<31))
yyerror("stack frame too large (>2GB)");
defframe(ptxt);
diff --git a/src/cmd/gc/reflect.c b/src/cmd/gc/reflect.c
index c8f8b39644..0847e9a3fb 100644
--- a/src/cmd/gc/reflect.c
+++ b/src/cmd/gc/reflect.c
@@ -907,7 +907,7 @@ dumptypestructs(void)
}
}
-Sym*
+static Sym*
dalgsym(Type *t)
{
int ot;
diff --git a/src/cmd/gc/subr.c b/src/cmd/gc/subr.c
index 55932ff3f0..3fd5209310 100644
--- a/src/cmd/gc/subr.c
+++ b/src/cmd/gc/subr.c
@@ -1108,11 +1108,9 @@ eqtype1(Type *t1, Type *t2, TypePairList *assumed_equal)
goto no;
yes:
-\tassumed_equal = l.next;
return 1;
no:
-\tassumed_equal = l.next;
return 0;
}
@@ -2491,7 +2489,7 @@ genwrapper(Type *rcvr, Type *method, Sym *newnam, int iface)
}
static Node*
-hashmem(Type *t, vlong width)
+hashmem(Type *t)
{
Node *tfn, *n;
Sym *sym;
@@ -2519,7 +2517,7 @@ hashfor(Type *t)
a = algtype1(t, nil);
switch(a) {
case AMEM:
-\t\treturn hashmem(t, t->width);
+\t\treturn hashmem(t);
case AINTER:
sym = pkglookup("interhash", runtimepkg);
break;
@@ -2667,7 +2665,7 @@ genhash(Sym *sym, Type *t)
size = t->width - first->width; // first->width is offset
else
size = t1->width - first->width; // both are offsets
-\t\t\thashel = hashmem(first->type, size);\
+\t\t\thashel = hashmem(first->type);\
// hashel(h, size, &p.first)
call = nod(OCALL, hashel, N);
call->list = list(call->list, nh);
diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c
index 0118c08a74..ea18766e30 100644
--- a/src/cmd/gc/walk.c
+++ b/src/cmd/gc/walk.c
@@ -1194,10 +1194,6 @@ walkexpr(Node **np, NodeList **init)
n = r;
goto ret;
-\t
-\t\tn = r;\
-\t\tgoto ret;\
-\
case OARRAYLIT:
case OMAPLIT:
case OSTRUCTLIT:
コアとなるコードの解説
src/cmd/8g/cgen.c
- 変更前:
print("\nsgen w=%ld\n", w);
- 変更後:
print("\nsgen w=%lld\n", w);
- 解説:
w
はint64
型(64ビット整数)ですが、%ld
は通常long
型(32ビットまたは64ビット、環境依存)のフォーマット指定子です。%lld
はlong long
型(64ビット整数)のフォーマット指定子であり、int64
型と正確に一致します。この修正により、w
の正しい値がデバッグ出力に表示されるようになり、コンパイラの警告が解消されました。
src/cmd/8l/asm.c
- 変更前:
resoff
変数が設定されているが、その後使用されていない。 - 変更後:
USED(resoff);
を追加。 - 解説:
resoff
変数がコード内で値が代入されているにもかかわらず、その後の処理で利用されていないため、コンパイラが「未使用変数」の警告を発していました。USED(resoff);
というマクロを追加することで、この変数が意図的に使用されていないことをコンパイラに明示的に伝え、警告を抑制します。これは、将来的にこの変数が使用される可能性を残しつつ、現在の警告を解消するための一般的な手法です。
src/cmd/gc/pgen.c
- 変更前:
if(stksize+maxarg > (1ULL<<31))
- 変更後:
if((int64)stksize+maxarg > (1ULL<<31))
- 解説:
stksize
とmaxarg
はスタックフレームのサイズに関連する変数で、vlong
型(64ビット整数)です。1ULL<<31
は符号なしのunsigned long long
型のリテラルで、値は2GBです。この比較では、stksize+maxarg
が1ULL<<31
よりも大きいかどうかをチェックしています。しかし、符号付き整数と符号なし整数を比較する際に、C言語の型変換ルールによって予期せぬ結果を招く可能性があります。特に、stksize+maxarg
が負の値になった場合、符号なし整数に変換されると非常に大きな値になり、比較が誤解を招く可能性があります。(int64)stksize+maxarg
と明示的にint64
にキャストすることで、比較が常に符号付き64ビット整数として行われるようになり、意図した通りの動作が保証され、警告が解消されました。
src/cmd/gc/reflect.c
- 変更前:
Sym* dalgsym(Type *t)
- 変更後:
static Sym* dalgsym(Type *t)
- 解説:
dalgsym
関数は、その定義されているファイル内でのみ使用される内部関数です。C言語では、関数にstatic
キーワードを付けることで、その関数のスコープを現在のファイルに限定し、他のファイルからのアクセスを防ぐことができます(内部リンケージ)。これにより、名前の衝突を防ぎ、カプセル化を促進します。この修正は、dalgsym
が前方宣言されている場合、その宣言と定義でstatic
の有無が一致していないとコンパイラが警告を発するため、それを解消するためのものです。
src/cmd/gc/subr.c
-
eqtype1
関数の変更:- 変更前:
yes:
とno:
のラベルの直前にassumed_equal = l.next;
という行があった。 - 変更後:
assumed_equal = l.next;
の行を削除。 - 解説:
eqtype1
関数はGoの型が等しいかどうかを再帰的に比較する関数で、assumed_equal
は無限ループを防ぐためのリストです。この修正では、assumed_equal = l.next;
という行が、goto
文によって到達しない、または不要なコードパスに存在していたため削除されました。これにより、未使用変数に関する警告が解消され、コードがクリーンになりました。
- 変更前:
-
hashmem
関数の変更:- 変更前:
static Node* hashmem(Type *t, vlong width)
- 変更後:
static Node* hashmem(Type *t)
- 解説:
hashmem
関数は、Goの型t
のメモリハッシュを計算するために使用されます。元の定義ではwidth
という引数も受け取っていましたが、この引数は関数内で実際には使用されていませんでした。未使用の引数はコンパイラの警告の原因となり、関数のインターフェースを不必要に複雑にします。この修正により、width
引数が削除され、それに伴いhashfor
やgenhash
からのhashmem
の呼び出しも引数を1つ減らすように変更されました。これにより、コードが簡素化され、警告が解消されました。
- 変更前:
src/cmd/gc/walk.c
- 変更前:
n = r; goto ret; n = r; goto ret;
- 変更後: 重複した
n = r; goto ret;
のブロックを削除。 - 解説:
walkexpr
関数内で、同じn = r; goto ret;
というコードブロックが2回連続して記述されていました。最初のブロックが実行されると、制御はret
ラベルにジャンプするため、2番目のブロックは決して実行されません。これは到達不能なコードであり、コードベースを不必要に複雑にし、保守を困難にします。この修正により、重複した到達不能なコードが削除され、コードがクリーンで理解しやすくなりました。
関連リンク
- Go CL 5651079: https://golang.org/cl/5651079
参考にした情報源リンク
- C言語の
static
キーワード: https://www.geeksforgeeks.org/static-keyword-in-c-cpp/ - C言語の
printf
フォーマット指定子: https://www.geeksforgeeks.org/format-specifiers-in-c/ - Goコンパイラの内部構造に関する一般的な情報 (Goの公式ドキュメントやブログ記事など)
- Goの
reflect
パッケージに関する情報 (Goの公式ドキュメントなど) - Goの
vlong
型に関する情報 (Goのソースコード内の定義など) USED
マクロの一般的な用途 (Linuxカーネルや他のCプロジェクトのソースコードなど)- Goの型比較に関する情報 (Goのソースコード内の
eqtype1
関数など) - Goのハッシュ関数に関する情報 (Goのソースコード内の
hashmem
関数など) - 抽象構文木 (AST) とコンパイラのウォーク処理に関する一般的な情報