From: Russ Cox Date: Sat, 2 Feb 2013 02:02:15 +0000 (-0500) Subject: cmd/gc: clearer error for defer/go of conversion or invalid function call X-Git-Tag: go1.1rc2~1188 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=79a16a3b70f0de2efbd45952a51e9b30524d7ad3;p=gostls13.git cmd/gc: clearer error for defer/go of conversion or invalid function call Fixes #4654. R=ken2 CC=golang-dev https://golang.org/cl/7229072 --- diff --git a/src/cmd/gc/const.c b/src/cmd/gc/const.c index 0224665519..a1621efa95 100644 --- a/src/cmd/gc/const.c +++ b/src/cmd/gc/const.c @@ -469,6 +469,20 @@ isconst(Node *n, int ct) return t == ct || (ct == CTINT && t == CTRUNE); } +static Node* +saveorig(Node *n) +{ + Node *n1; + + if(n == n->orig) { + // duplicate node for n->orig. + n1 = nod(OLITERAL, N, N); + n->orig = n1; + *n1 = *n; + } + return n->orig; +} + /* * if n is constant, rewrite as OLITERAL node. */ @@ -934,15 +948,14 @@ unary: } ret: - if(n == n->orig) { - // duplicate node for n->orig. - norig = nod(OLITERAL, N, N); - *norig = *n; - } else - norig = n->orig; + norig = saveorig(n); *n = *nl; // restore value of n->orig. n->orig = norig; + if(norig->op == OCONV) { + dump("N", n); + dump("NORIG", norig); + } n->val = v; // check range. @@ -956,11 +969,15 @@ ret: return; settrue: + norig = saveorig(n); *n = *nodbool(1); + n->orig = norig; return; setfalse: + norig = saveorig(n); *n = *nodbool(0); + n->orig = norig; return; } diff --git a/src/cmd/gc/fmt.c b/src/cmd/gc/fmt.c index ce6ee729bd..53a200b4fe 100644 --- a/src/cmd/gc/fmt.c +++ b/src/cmd/gc/fmt.c @@ -381,7 +381,13 @@ Vconv(Fmt *fp) case CTCPLX: if((fp->flags & FmtSharp) || fmtmode == FExp) return fmtprint(fp, "(%F+%Fi)", &v->u.cval->real, &v->u.cval->imag); - return fmtprint(fp, "(%#F + %#Fi)", &v->u.cval->real, &v->u.cval->imag); + if(mpcmpfltc(&v->u.cval->real, 0) == 0) + return fmtprint(fp, "%#Fi", &v->u.cval->imag); + if(mpcmpfltc(&v->u.cval->imag, 0) == 0) + return fmtprint(fp, "%#F", &v->u.cval->real); + if(mpcmpfltc(&v->u.cval->imag, 0) < 0) + return fmtprint(fp, "(%#F%#Fi)", &v->u.cval->real, &v->u.cval->imag); + return fmtprint(fp, "(%#F+%#Fi)", &v->u.cval->real, &v->u.cval->imag); case CTSTR: return fmtprint(fp, "\"%Z\"", v->u.sval); case CTBOOL: @@ -391,7 +397,7 @@ Vconv(Fmt *fp) case CTNIL: return fmtstrcpy(fp, "nil"); } - return fmtprint(fp, "<%d>", v->ctype); + return fmtprint(fp, "", v->ctype); } // Fmt "%Z": escaped string literals @@ -1110,8 +1116,8 @@ exprfmt(Fmt *f, Node *n, int prec) case OLITERAL: // this is a bit of a mess if(fmtmode == FErr && n->sym != S) return fmtprint(f, "%S", n->sym); - if(n->val.ctype == CTNIL && n->orig != N) - n = n->orig; // if this node was a nil decorated with at type, print the original naked nil + if(n->val.ctype == CTNIL && n->orig != N && n->orig != n) + return exprfmt(f, n->orig, prec); if(n->type != T && n->type != types[n->type->etype] && n->type != idealbool && n->type != idealstring) { // Need parens when type begins with what might // be misinterpreted as a unary operator: * or <-. diff --git a/src/cmd/gc/typecheck.c b/src/cmd/gc/typecheck.c index 3846f7b8f1..5cc398c8d0 100644 --- a/src/cmd/gc/typecheck.c +++ b/src/cmd/gc/typecheck.c @@ -31,6 +31,7 @@ static void checkassign(Node*); static void checkassignlist(NodeList*); static void stringtoarraylit(Node**); static Node* resolve(Node*); +static void checkdefergo(Node*); static NodeList* typecheckdefstack; @@ -1122,10 +1123,12 @@ reswitch: if(!iscomplex[t->etype]) goto badcall1; if(isconst(l, CTCPLX)){ + r = n; if(n->op == OREAL) n = nodfltconst(&l->val.u.cval->real); else n = nodfltconst(&l->val.u.cval->imag); + n->orig = r; } n->type = types[cplxsubtype(t->etype)]; goto ret; @@ -1185,7 +1188,9 @@ reswitch: } if(l->op == OLITERAL && r->op == OLITERAL) { // make it a complex literal - n = nodcplxlit(l->val, r->val); + r = nodcplxlit(l->val, r->val); + r->orig = n; + n = r; } n->type = t; goto ret; @@ -1336,6 +1341,10 @@ reswitch: switch(n->op) { case OCONVNOP: if(n->left->op == OLITERAL) { + r = nod(OXXX, N, N); + n->op = OCONV; + n->orig = r; + *r = *n; n->op = OLITERAL; n->val = n->left->val; } @@ -1539,12 +1548,15 @@ reswitch: case ODEFER: ok |= Etop; - typecheck(&n->left, Etop); + typecheck(&n->left, Etop|Erv); + if(!n->left->diag) + checkdefergo(n); goto ret; case OPROC: ok |= Etop; - typecheck(&n->left, Etop|Eproc); + typecheck(&n->left, Etop|Eproc|Erv); + checkdefergo(n); goto ret; case OFOR: @@ -1664,7 +1676,7 @@ ret: } if((top & Etop) && !(top & (Ecall|Erv|Etype)) && !(ok & Etop)) { if(n->diag == 0) { - yyerror("%N not used", n); + yyerror("%N evaluated but not used", n); n->diag = 1; } goto error; @@ -1687,6 +1699,56 @@ out: *np = n; } +static void +checkdefergo(Node *n) +{ + char *what; + + what = "defer"; + if(n->op == OPROC) + what = "go"; + + switch(n->left->op) { + case OCALLINTER: + case OCALLMETH: + case OCALLFUNC: + case OCLOSE: + case OCOPY: + case ODELETE: + case OPANIC: + case OPRINT: + case OPRINTN: + case ORECOVER: + // ok + break; + case OAPPEND: + case OCAP: + case OCOMPLEX: + case OIMAG: + case OLEN: + case OMAKE: + case OMAKESLICE: + case OMAKECHAN: + case OMAKEMAP: + case ONEW: + case OREAL: + case OLITERAL: // conversion or unsafe.Alignof, Offsetof, Sizeof + if(n->left->orig != N && n->left->orig->op == OCONV) + goto conv; + yyerror("%s discards result of %N", what, n->left); + break; + default: + conv: + if(!n->diag) { + // The syntax made sure it was a call, so this must be + // a conversion. + n->diag = 1; + yyerror("%s requires function call, not conversion", what); + } + break; + } +} + static void implicitstar(Node **nn) { diff --git a/test/fixedbugs/issue4463.go b/test/fixedbugs/issue4463.go index 578173aba5..fe07af71fb 100644 --- a/test/fixedbugs/issue4463.go +++ b/test/fixedbugs/issue4463.go @@ -45,17 +45,17 @@ func F() { (println("bar")) (recover()) - go append(a, 0) // ERROR "not used" - go cap(a) // ERROR "not used" - go complex(1, 2) // ERROR "not used" - go imag(1i) // ERROR "not used" - go len(a) // ERROR "not used" - go make([]int, 10) // ERROR "not used" - go new(int) // ERROR "not used" - go real(1i) // ERROR "not used" - go unsafe.Alignof(a) // ERROR "not used" - go unsafe.Offsetof(s.f) // ERROR "not used" - go unsafe.Sizeof(a) // ERROR "not used" + go append(a, 0) // ERROR "discards result" + go cap(a) // ERROR "discards result" + go complex(1, 2) // ERROR "discards result" + go imag(1i) // ERROR "discards result" + go len(a) // ERROR "discards result" + go make([]int, 10) // ERROR "discards result" + go new(int) // ERROR "discards result" + go real(1i) // ERROR "discards result" + go unsafe.Alignof(a) // ERROR "discards result" + go unsafe.Offsetof(s.f) // ERROR "discards result" + go unsafe.Sizeof(a) // ERROR "discards result" go close(c) go copy(a, a) @@ -65,17 +65,17 @@ func F() { go println("bar") go recover() - defer append(a, 0) // ERROR "not used" - defer cap(a) // ERROR "not used" - defer complex(1, 2) // ERROR "not used" - defer imag(1i) // ERROR "not used" - defer len(a) // ERROR "not used" - defer make([]int, 10) // ERROR "not used" - defer new(int) // ERROR "not used" - defer real(1i) // ERROR "not used" - defer unsafe.Alignof(a) // ERROR "not used" - defer unsafe.Offsetof(s.f) // ERROR "not used" - defer unsafe.Sizeof(a) // ERROR "not used" + defer append(a, 0) // ERROR "discards result" + defer cap(a) // ERROR "discards result" + defer complex(1, 2) // ERROR "discards result" + defer imag(1i) // ERROR "discards result" + defer len(a) // ERROR "discards result" + defer make([]int, 10) // ERROR "discards result" + defer new(int) // ERROR "discards result" + defer real(1i) // ERROR "discards result" + defer unsafe.Alignof(a) // ERROR "discards result" + defer unsafe.Offsetof(s.f) // ERROR "discards result" + defer unsafe.Sizeof(a) // ERROR "discards result" defer close(c) defer copy(a, a) diff --git a/test/fixedbugs/issue4654.go b/test/fixedbugs/issue4654.go new file mode 100644 index 0000000000..ede7f56e7f --- /dev/null +++ b/test/fixedbugs/issue4654.go @@ -0,0 +1,71 @@ +// errorcheck + +// Copyright 2013 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Issue 4654. +// Check error for conversion and 'not used' in defer/go. + +package p + +import "unsafe" + +func f() { + defer int(0) // ERROR "defer requires function call, not conversion" + go string([]byte("abc")) // ERROR "go requires function call, not conversion" + + var c complex128 + var f float64 + var t struct {X int} + + var x []int + defer append(x, 1) // ERROR "defer discards result of append" + defer cap(x) // ERROR "defer discards result of cap" + defer complex(1, 2) // ERROR "defer discards result of complex" + defer complex(f, 1) // ERROR "defer discards result of complex" + defer imag(1i) // ERROR "defer discards result of imag" + defer imag(c) // ERROR "defer discards result of imag" + defer len(x) // ERROR "defer discards result of len" + defer make([]int, 1) // ERROR "defer discards result of make" + defer make(chan bool) // ERROR "defer discards result of make" + defer make(map[string]int) // ERROR "defer discards result of make" + defer new(int) // ERROR "defer discards result of new" + defer real(1i) // ERROR "defer discards result of real" + defer real(c) // ERROR "defer discards result of real" + defer append(x, 1) // ERROR "defer discards result of append" + defer append(x, 1) // ERROR "defer discards result of append" + defer unsafe.Alignof(t.X) // ERROR "defer discards result of unsafe.Alignof" + defer unsafe.Offsetof(t.X) // ERROR "defer discards result of unsafe.Offsetof" + defer unsafe.Sizeof(t) // ERROR "defer discards result of unsafe.Sizeof" + + defer copy(x, x) // ok + m := make(map[int]int) + defer delete(m, 1) // ok + defer panic(1) // ok + defer print(1) // ok + defer println(1) // ok + defer recover() // ok + + int(0) // ERROR "int\(0\) not used" + string([]byte("abc")) // ERROR "string\(\[\]byte literal\) not used" + + append(x, 1) // ERROR "not used" + cap(x) // ERROR "not used" + complex(1, 2) // ERROR "not used" + complex(f, 1) // ERROR "not used" + imag(1i) // ERROR "not used" + imag(c) // ERROR "not used" + len(x) // ERROR "not used" + make([]int, 1) // ERROR "not used" + make(chan bool) // ERROR "not used" + make(map[string]int) // ERROR "not used" + new(int) // ERROR "not used" + real(1i) // ERROR "not used" + real(c) // ERROR "not used" + append(x, 1) // ERROR "not used" + append(x, 1) // ERROR "not used" + unsafe.Alignof(t.X) // ERROR "not used" + unsafe.Offsetof(t.X) // ERROR "not used" + unsafe.Sizeof(t) // ERROR "not used" +}