Skip to content

Commit

Permalink
Merge pull request #20 from ldez/fix/code-review
Browse files Browse the repository at this point in the history 
Quick code review
  • Loading branch information
@sanposhiho
sanposhiho committed Feb 27, 2021
2 parents f076c45 + ddf617f commit 1413d2f
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 48 deletions.
3 changes: 0 additions & 3 deletions go.sum
View file
@@ -1,8 +1,6 @@
github.com/yuin/goldmark v1.2.1 h1:ruQGxdhGHe7FWOJPT0mKs5+pD2Xs1Bm/kdGlHO04FmM=
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 h1:psW17arqaxU48Z5kZ0CQnkZWQJsqcURM6tKiBApRjXI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
Expand All @@ -16,7 +14,6 @@ golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
Expand Down
91 changes: 46 additions & 45 deletions wastedassign.go
View file
@@ -1,7 +1,7 @@
package wastedassign

import (
"fmt"
"errors"
"go/ast"
"go/token"
"go/types"
Expand All @@ -14,7 +14,7 @@ import (

const doc = "wastedassign finds wasted assignment statements."

// Analyzer is ...
// Analyzer is the wastedassign analyzer.
var Analyzer = &analysis.Analyzer{
Name: "wastedassign",
Doc: doc,
Expand All @@ -31,8 +31,7 @@ type wastedAssignStruct struct {

func run(pass *analysis.Pass) (interface{}, error) {
// Plundered from buildssa.Run.
mode := ssa.NaiveForm
prog := ssa.NewProgram(pass.Fset, mode)
prog := ssa.NewProgram(pass.Fset, ssa.NaiveForm)

// Create SSA packages for all imports.
// Order is not significant.
Expand Down Expand Up @@ -73,12 +72,12 @@ func run(pass *analysis.Pass) (interface{}, error) {

fn := pass.TypesInfo.Defs[fdecl.Name].(*types.Func)
if fn == nil {
return nil, fmt.Errorf("failed to get func's typesinfo")
return nil, errors.New("failed to get func's typesinfo")
}

f := ssapkg.Prog.FuncValue(fn)
if f == nil {
return nil, fmt.Errorf("failed to get func's SSA-form intermediate representation")
return nil, errors.New("failed to get func's SSA-form intermediate representation")
}

var addAnons func(f *ssa.Function)
Expand All @@ -96,41 +95,50 @@ func run(pass *analysis.Pass) (interface{}, error) {
typeSwitchPos := map[int]bool{}
inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
inspect.Preorder([]ast.Node{new(ast.TypeSwitchStmt)}, func(n ast.Node) {
switch n := n.(type) {
case *ast.TypeSwitchStmt:
if _, ok := n.(*ast.TypeSwitchStmt); ok {
typeSwitchPos[pass.Fset.Position(n.Pos()).Line] = true
}
})

wastedAssignMap := []wastedAssignStruct{}
var wastedAssignMap []wastedAssignStruct

for _, sf := range srcFuncs {
for _, bl := range sf.Blocks {
blCopy := *bl
for _, ist := range bl.Instrs {
blCopy.Instrs = rmInstrFromInstrs(blCopy.Instrs, ist)
switch ist.(type) {
case *ssa.Store:
var buf [10]*ssa.Value
for _, op := range ist.Operands(buf[:0]) {
if (*op) != nil && opInLocals(sf.Locals, op) {
if reason := isNextOperationToOpIsStore([]*ssa.BasicBlock{&blCopy}, op, nil); reason != notWasted {
if ist.Pos() != 0 && !typeSwitchPos[pass.Fset.Position(ist.Pos()).Line] {
wastedAssignMap = append(wastedAssignMap, wastedAssignStruct{
pos: ist.Pos(),
reason: reason.String(),
})
}
}
}
if _, ok := ist.(*ssa.Store); !ok {
continue
}

var buf [10]*ssa.Value
for _, op := range ist.Operands(buf[:0]) {
if (*op) == nil || !opInLocals(sf.Locals, op) {
continue
}

reason := isNextOperationToOpIsStore([]*ssa.BasicBlock{&blCopy}, op, nil)
if reason == notWasted {
continue
}

if ist.Pos() == 0 || typeSwitchPos[pass.Fset.Position(ist.Pos()).Line] {
continue
}

wastedAssignMap = append(wastedAssignMap, wastedAssignStruct{
pos: ist.Pos(),
reason: reason.String(),
})
}
}
}
}

for _, was := range wastedAssignMap {
pass.Reportf(was.pos, was.reason)
}

return nil, nil
}

Expand All @@ -150,28 +158,31 @@ func (wr wastedReason) String() string {
return "wasted assignment"
case notWasted:
return ""
default:
return ""
}
return ""
}

func isNextOperationToOpIsStore(bls []*ssa.BasicBlock, currentOp *ssa.Value, haveCheckedMap *map[int]bool) wastedReason {
wastedReasons := []wastedReason{}
wastedReasonsCurrentBls := []wastedReason{}
func isNextOperationToOpIsStore(bls []*ssa.BasicBlock, currentOp *ssa.Value, haveCheckedMap map[int]bool) wastedReason {
var wastedReasons []wastedReason
var wastedReasonsCurrentBls []wastedReason

if haveCheckedMap == nil {
haveCheckedMap = &map[int]bool{}
haveCheckedMap = map[int]bool{}
}

for _, bl := range bls {
if (*haveCheckedMap)[bl.Index] == true {
if haveCheckedMap[bl.Index] {
continue
}
(*haveCheckedMap)[bl.Index] = true

haveCheckedMap[bl.Index] = true
breakFlag := false
for _, ist := range bl.Instrs {
if breakFlag {
break
}

switch w := ist.(type) {
case *ssa.Store:
var buf [10]*ssa.Value
Expand All @@ -196,6 +207,7 @@ func isNextOperationToOpIsStore(bls []*ssa.BasicBlock, currentOp *ssa.Value, hav
}
}
}

if len(bl.Succs) != 0 && !breakFlag {
wastedReason := isNextOperationToOpIsStore(rmSameBlock(bl.Succs, bl), currentOp, haveCheckedMap)
if wastedReason == notWasted {
Expand All @@ -207,17 +219,15 @@ func isNextOperationToOpIsStore(bls []*ssa.BasicBlock, currentOp *ssa.Value, hav

wastedReasons = append(wastedReasons, wastedReasonsCurrentBls...)

if len(wastedReasons) != 0 {
if containReassignedSoon(wastedReasons) {
return reassignedSoon
}
return noUseUntilReturn
if len(wastedReasons) != 0 && containReassignedSoon(wastedReasons) {
return reassignedSoon
}

return noUseUntilReturn
}

func rmSameBlock(bls []*ssa.BasicBlock, currentBl *ssa.BasicBlock) []*ssa.BasicBlock {
rto := []*ssa.BasicBlock{}
var rto []*ssa.BasicBlock

for _, bl := range bls {
if bl != currentBl {
Expand All @@ -227,15 +237,6 @@ func rmSameBlock(bls []*ssa.BasicBlock, currentBl *ssa.BasicBlock) []*ssa.BasicB
return rto
}

func containNotWasted(ws []wastedReason) bool {
for _, w := range ws {
if w == notWasted {
return true
}
}
return false
}

func containReassignedSoon(ws []wastedReason) bool {
for _, w := range ws {
if w == reassignedSoon {
Expand Down

0 comments on commit 1413d2f

Please sign in to comment.