From 664ef9a3e1016e80fc1fcbbef6e9f66a2ededfe6 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Sat, 30 Jun 2018 19:34:41 +0200 Subject: pkg/compiler: check for unused declarations Error on unused structs/unions/resources/flags. Finds tons of bugs. --- pkg/compiler/check.go | 79 ++++++++++++++++++++++++++++++++------- pkg/compiler/compiler.go | 17 +++++---- pkg/compiler/consts_test.go | 2 +- pkg/compiler/testdata/all.txt | 13 +++++++ pkg/compiler/testdata/consts.txt | 3 ++ pkg/compiler/testdata/errors2.txt | 35 +++++++++++++++++ 6 files changed, 127 insertions(+), 22 deletions(-) (limited to 'pkg') diff --git a/pkg/compiler/check.go b/pkg/compiler/check.go index 496490dff..19e196adb 100644 --- a/pkg/compiler/check.go +++ b/pkg/compiler/check.go @@ -23,7 +23,7 @@ func (comp *compiler) typecheck() { func (comp *compiler) check() { comp.checkTypeValues() comp.checkAttributeValues() - comp.checkUsed() + comp.checkUnused() comp.checkRecursion() comp.checkLenTargets() comp.checkConstructors() @@ -94,6 +94,9 @@ func (comp *compiler) checkNames() { } case *ast.IntFlags: name := n.Name.Name + if name == "_" { + continue + } if reservedName[name] { comp.error(n.Pos, "flags uses reserved name %v", name) continue @@ -359,48 +362,95 @@ func (comp *compiler) checkLenTarget(t *ast.Type, name, target string, fields [] comp.error(t.Pos, "%v target %v does not exist", t.Ident, target) } -func (comp *compiler) checkUsed() { +func (comp *compiler) collectUsed(all bool) (structs, flags, strflags map[string]bool) { + structs = make(map[string]bool) + flags = make(map[string]bool) + strflags = make(map[string]bool) for _, decl := range comp.desc.Nodes { switch n := decl.(type) { case *ast.Call: - if n.NR == ^uint64(0) { + if !all && n.NR == ^uint64(0) { break } for _, arg := range n.Args { - comp.checkUsedType(arg.Type, true) + comp.collectUsedType(structs, flags, strflags, arg.Type, true) } if n.Ret != nil { - comp.checkUsedType(n.Ret, true) + comp.collectUsedType(structs, flags, strflags, n.Ret, true) } } } + return } -func (comp *compiler) checkUsedType(t *ast.Type, isArg bool) { - if comp.used[t.Ident] { - return - } +func (comp *compiler) collectUsedType(structs, flags, strflags map[string]bool, t *ast.Type, isArg bool) { desc := comp.getTypeDesc(t) if desc == typeResource { r := comp.resources[t.Ident] - for r != nil && !comp.used[r.Name.Name] { - comp.used[r.Name.Name] = true + for r != nil && !structs[r.Name.Name] { + structs[r.Name.Name] = true r = comp.resources[r.Base.Ident] } return } if desc == typeStruct { - comp.used[t.Ident] = true + if structs[t.Ident] { + return + } + structs[t.Ident] = true s := comp.structs[t.Ident] for _, fld := range s.Fields { - comp.checkUsedType(fld.Type, false) + comp.collectUsedType(structs, flags, strflags, fld.Type, false) + } + return + } + if desc == typeFlags { + flags[t.Args[0].Ident] = true + return + } + if desc == typeString { + if len(t.Args) != 0 && t.Args[0].Ident != "" { + strflags[t.Args[0].Ident] = true } return } _, args, _ := comp.getArgsBase(t, "", prog.DirIn, isArg) for i, arg := range args { if desc.Args[i].Type == typeArgType { - comp.checkUsedType(arg, false) + comp.collectUsedType(structs, flags, strflags, arg, false) + } + } +} + +func (comp *compiler) checkUnused() { + comp.used, _, _ = comp.collectUsed(false) + structs, flags, strflags := comp.collectUsed(true) + _, _, _ = structs, flags, strflags + + for name, n := range comp.intFlags { + if !flags[name] { + comp.error(n.Pos, "unused flags %v", name) + } + } + for name, n := range comp.strFlags { + if !strflags[name] && builtinStrFlags[name] == nil { + comp.error(n.Pos, "unused string flags %v", name) + } + } + for name, n := range comp.resources { + if !structs[name] { + comp.error(n.Pos, "unused resource %v", name) + } + } + for name, n := range comp.structs { + if !structs[name] { + _, typ, _ := n.Info() + comp.error(n.Pos, "unused %v %v", typ, name) + } + } + for name, n := range comp.typedefs { + if !comp.usedTypedefs[name] { + comp.error(n.Pos, "unused type %v", name) } } } @@ -697,6 +747,7 @@ func (comp *compiler) checkType(ctx checkCtx, t *ast.Type, flags checkFlags) { func (comp *compiler) replaceTypedef(ctx *checkCtx, t *ast.Type, flags checkFlags) { typedefName := t.Ident + comp.usedTypedefs[typedefName] = true if t.HasColon { comp.error(t.Pos, "type alias %v with ':'", t.Ident) return diff --git a/pkg/compiler/compiler.go b/pkg/compiler/compiler.go index f49576561..b44a698ca 100644 --- a/pkg/compiler/compiler.go +++ b/pkg/compiler/compiler.go @@ -60,12 +60,14 @@ func Compile(desc *ast.Description, consts map[string]uint64, target *targets.Ta intFlags: make(map[string]*ast.IntFlags), strFlags: make(map[string]*ast.StrFlags), used: make(map[string]bool), + usedTypedefs: make(map[string]bool), structDescs: make(map[prog.StructKey]*prog.StructDesc), structNodes: make(map[*prog.StructDesc]*ast.Struct), structVarlen: make(map[string]bool), } for name, n := range builtinTypedefs { comp.typedefs[name] = n + comp.usedTypedefs[name] = true } for name, n := range builtinStrFlags { comp.strFlags[name] = n @@ -116,13 +118,14 @@ type compiler struct { warnings []warn ptrSize uint64 - unsupported map[string]bool - resources map[string]*ast.Resource - typedefs map[string]*ast.TypeDef - structs map[string]*ast.Struct - intFlags map[string]*ast.IntFlags - strFlags map[string]*ast.StrFlags - used map[string]bool // contains used structs/resources + unsupported map[string]bool + resources map[string]*ast.Resource + typedefs map[string]*ast.TypeDef + structs map[string]*ast.Struct + intFlags map[string]*ast.IntFlags + strFlags map[string]*ast.StrFlags + used map[string]bool // contains used structs/resources + usedTypedefs map[string]bool structDescs map[prog.StructKey]*prog.StructDesc structNodes map[*prog.StructDesc]*ast.Struct diff --git a/pkg/compiler/consts_test.go b/pkg/compiler/consts_test.go index 522062991..37c2625d1 100644 --- a/pkg/compiler/consts_test.go +++ b/pkg/compiler/consts_test.go @@ -37,7 +37,7 @@ func TestExtractConsts(t *testing.T) { "CONST6", "CONST7", "CONST8", "CONST9", "CONST10", "CONST11", "CONST12", "CONST13", "CONST14", "CONST15", "CONST16", "CONST17", "CONST18", "CONST19", "CONST20", - "CONST21", + "CONST21", "CONST22", "CONST23", "CONST24", } sort.Strings(wantConsts) if !reflect.DeepEqual(info.Consts, wantConsts) { diff --git a/pkg/compiler/testdata/all.txt b/pkg/compiler/testdata/all.txt index 69ad6d181..a5a097de3 100644 --- a/pkg/compiler/testdata/all.txt +++ b/pkg/compiler/testdata/all.txt @@ -7,6 +7,7 @@ foo$2(a ptr[out, array[int32]]) foo$3(a union_arg) foo$4() r0 foo$5(a int8['a':'z']) +foo$6(a ptr[in, strings]) resource r0[intptr] @@ -38,6 +39,8 @@ strings { string_flags1 = "foo", "barbaz" string_flags2 = "" int_flags = 0, 1 +_ = 1, 2 +_ = C1, C2 # Proc type. @@ -45,6 +48,8 @@ proc_struct1 { f1 proc[C0, 8, int8] } +foo$proc1(a ptr[in, proc_struct1]) + # Len/bytesize types. type len_templ1[DATA1, DATA2] { @@ -122,6 +127,8 @@ bitfield0 { f2 int8:2 } +foo$bitfield0(a ptr[in, bitfield0]) + # Type templates. type type0 int8 @@ -184,6 +191,8 @@ foo$templ3(a ptr[in, templ_struct1[1]], b ptr[in, templ_struct1[2]]) foo$templ4(a ptr[in, templ_struct1[3]]) foo$templ5(a ptr[in, templ_struct1[3]]) foo$templ6(a ptr[in, templ_struct4]) +foo$templ7(a ptr[in, templ_struct5], b ptr[in, templ_struct6], c ptr[in, templ_union], d ptr[in, type3]) +foo$templ8(a ptr[in, templ_templ_use]) # Structs. @@ -195,8 +204,12 @@ s1 { f1 int8 } [size[C2]] +foo$s0(a ptr[in, s0], b ptr[in, s1]) + # Unions. u0 [ f1 int32 ] + +foo$u0(a ptr[in, u0]) diff --git a/pkg/compiler/testdata/consts.txt b/pkg/compiler/testdata/consts.txt index f7c8daa35..ef248a1e0 100644 --- a/pkg/compiler/testdata/consts.txt +++ b/pkg/compiler/testdata/consts.txt @@ -35,3 +35,6 @@ foo$1(a ptr[in, templ1[CONST20]]) str2 { f1 int8 } [size[CONST21]] + +_ = CONST22, CONST23 +_ = CONST24 diff --git a/pkg/compiler/testdata/errors2.txt b/pkg/compiler/testdata/errors2.txt index 69fa46e62..1b1adefa5 100644 --- a/pkg/compiler/testdata/errors2.txt +++ b/pkg/compiler/testdata/errors2.txt @@ -6,6 +6,7 @@ resource r0[r0] ### recursive resource r0->r0 resource r1[r2] ### recursive resource r1->r2->r1 resource r2[r1] ### recursive resource r2->r1->r2 +resource r3[int32] ### unused resource r3 foo$0(a0 ptr[out, r0], a1 ptr[out, r1], a2 ptr[out, r2]) @@ -54,6 +55,28 @@ sr9 { f templ_sr[ptr[in, sr9]] ### recursive declaration: sr9.f -> templ_sr[ptr[in, sr9]].f -> sr9 (mark some pointers as opt) } +use_sr { + u2 u2 + u3 u3 + s3 s3 + s4 s4 + s6 s6 + sr1 sr1 + sr2 sr2 + sr5 sr5 + sr7 sr7 + sr8 sr8 + sr9 sr9 + s400 s400 + s401 s401 + u400 u400 + u401 u401 + s402 s402 + s403 s403 +} [packed] + +foo$sr0(a ptr[in, use_sr]) + # Len target tests. foo$100(a int32, b len[a]) @@ -169,8 +192,17 @@ s403 { f3 int32 } +s404 { ### unused struct s404 + f1 int8 +} + +u404 [ ### unused union u404 + f1 int8 +] + sf400 = "foo", "bar", "baz" sf401 = "a", "b", "cd" +sf402 = "a", "b" ### unused string flags sf402 # Const argument checks. @@ -184,5 +216,8 @@ foo$506(a ptr[in, array[int32, 0]]) ### arrays of size 0 are not supported foo$507(a ptr[in, array[int32, 0:0]]) ### arrays of size 0 are not supported foo$508(a ptr[in, string["foo", 3]]) ### string value "foo\x00" exceeds buffer length 3 foo$509(a int8['b':'a']) ### bad int range [98:97] +foo$510(a type500) type type500 proc[C1, 8, int8] ### values starting from 1 with step 8 overflow base type for 32 procs +type type501 int8 ### unused type type501 +type type502[C] const[C, int8] ### unused type type502 -- cgit mrf-deployment