Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Quick code review #20

Merged
merged 2 commits into from
Feb 27, 2021
Merged

Quick code review #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c2f5ec
go mod tidy
ldez Feb 21, 2021
ddf617f
code review.
ldez Feb 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 0 additions & 3 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Open in desktop
Original file line number Diff line number Diff line change
@@ -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