[インデックス 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) とコンパイラのウォーク処理に関する一般的な情報