search

mdempsky / unconvert

Remove unnecessary type conversions from Go source
BSD 3-Clause "New" or "Revised" License
377 stars 26 forks source link

fails on cockroachdb #11

Closed tamird closed 8 years ago

tamird commented 8 years ago

Cool tool!

I tried it on cockroachdb/cockroach@ac79461:

$ unconvert -all -apply ./...
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/util/log/clog.go:949:40: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/internal/client/txn.go:353:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/internal/client/txn.go:354:8: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/internal/client/db.go:484:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/sql/parser/builtins.go:988:23: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/status/recorder.go:260:18: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/in_mem.go:26:3: undeclared name: RocksDB
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/in_mem.go:44:16: cannot use (InMem literal) (value of type InMem) as Engine value in variable declaration: missing method ApplyBatchRepr
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/in_mem.go:31:11: undeclared name: NewRocksDBCache
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/in_mem.go:36:3: unknown field RocksDB in struct literal
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/in_mem.go:38:12: invalid operation: db (variable of type InMem) has no field or method Open
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/merge.go:56:22: undeclared name: goMerge
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/mvcc.go:551:20: undeclared name: emptyKeyError
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/mvcc.go:950:10: undeclared name: emptyKeyError
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/mvcc.go:1289:10: undeclared name: emptyKeyError
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/mvcc.go:1506:15: undeclared name: emptyKeyError
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/mvcc.go:1716:10: undeclared name: emptyKeyError
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/kv/dist_sender.go:342:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/kv/dist_sender.go:343:8: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/kv/dist_sender.go:491:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/store.go:1151:19: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/store.go:2859:31: RocksDB not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/store.go:2859:20: s.engine (variable of type github.com/cockroachdb/cockroach/storage/engine.Engine) cannot have dynamic type *invalid type (missing method ApplyBatchRepr)
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/store.go:2860:15: invalid operation: rocksdb (variable of type *invalid type) has no field or method GetSSTables
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/replica_command.go:2121:18: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/testutils/localtestcluster/local_test_cluster.go:92:12: cannot use engine.NewInMem((roachpb.Attributes literal), 50 << 20, ltc.Stopper) (value of type github.com/cockroachdb/cockroach/storage/engine.InMem) as github.com/cockroachdb/cockroach/storage/engine.Engine value in assignment: missing method ApplyBatchRepr
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/sql/scan.go:184:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/sql/plan.go:225:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/sql/update.go:130:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/sql/update.go:180:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/sql/update.go:299:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/sql/pgwire/v3.go:639:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/sql/pgwire/v3.go:642:2: AnnotateTrace not declared by package tracing
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:339:11: NewRocksDBCache not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:367:38: cannot use engine.NewInMem(spec.Attributes, sizeInBytes, stopper) (value of type github.com/cockroachdb/cockroach/storage/engine.InMem) as github.com/cockroachdb/cockroach/storage/engine.Engine value in argument to append: missing method ApplyBatchRepr
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:382:5: NewRocksDB not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/status.go:240:15: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/updates.go:171:19: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/updates.go:272:19: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/admin.go:850:55: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:226:52: MinimumMaxOpenFiles not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:227:59: DefaultMaxOpenFiles not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:228:56: DefaultMaxOpenFiles not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:233:105: DefaultMaxOpenFiles not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:235:10: DefaultMaxOpenFiles not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:249:10: DefaultMaxOpenFiles not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:289:10: DefaultMaxOpenFiles not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/context.go:308:10: DefaultMaxOpenFiles not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/server/testserver.go:224:43: cannot use engine.NewInMem((roachpb.Attributes literal), 100 << 20, params.Stopper) (value of type github.com/cockroachdb/cockroach/storage/engine.InMem) as github.com/cockroachdb/cockroach/storage/engine.Engine value in argument to append: missing method ApplyBatchRepr
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/cli.go:42:11: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:65:73: RocksDB not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/backtrace.go:69:10: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:139:12: invalid operation: db (variable of type *invalid type) has no field or method Iterate
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:66:11: NewRocksDBCache not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:68:8: NewRocksDB not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:74:3: DefaultMaxOpenFiles not declared by package engine
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:174:35: cannot use db (variable of type *invalid type) as github.com/cockroachdb/cockroach/storage/engine.Engine value in argument to loadRangeDescriptor: missing method ApplyBatchRepr
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:179:48: cannot use db (variable of type *invalid type) as github.com/cockroachdb/cockroach/storage/engine.Reader value in argument to storage.NewReplicaDataIterator: missing method Close
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:405:12: invalid operation: db (variable of type *invalid type) has no field or method Iterate
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:485:12: invalid operation: db (variable of type *invalid type) has no field or method Iterate
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:532:56: cannot use db (variable of type *invalid type) as github.com/cockroachdb/cockroach/storage/engine.Reader value in argument to engine.MVCCIterate: missing method Close
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:559:11: invalid operation: db (variable of type *invalid type) has no field or method NewSnapshot
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:618:56: cannot use db (variable of type *invalid type) as github.com/cockroachdb/cockroach/storage/engine.Reader value in argument to engine.MVCCIterate: missing method Close
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:710:9: invalid operation: db (variable of type *invalid type) has no field or method Compact
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/debug.go:753:19: invalid operation: db (variable of type *invalid type) has no field or method GetSSTables
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/gen.go:44:10: GetInfo not declared by package build
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/cli/start.go:333:10: GetInfo not declared by package build
2016/07/28 22:14:02 couldn't load packages due to errors: github.com/cockroachdb/cockroach/cli, github.com/cockroachdb/cockroach/storage/engine, github.com/cockroachdb/cockroach/testutils/localtestcluster and 9 more
mdempsky commented 8 years ago

It looks like the issue you're running into is that build.GetInfo is declared in a file that uses cgo, and go/types doesn't really support cgo. Particularly not with -all, because you would need C cross-compilers for all target platforms.

I think your two options are:

  1. Run without -all, and then unconvert will enable cgo (at least according to CGO_ENABLED).
  2. Change package build (and possibly others) to work (or at least compile) without cgo.

I'm going to close since I think there's unfortunately nothing unconvert can do here, but happy to reopen if I'm overlooking anything.

Cheers!

tamird commented 8 years ago

Thanks for the reply. Even without -all, things break in the presence of cgo, it seems?

$ unconvert ./...
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:364:22: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:374:8: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:512:4: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:514:23: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:588:14: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:796:2: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:825:32: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:855:4: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:889:27: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:917:17: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:984:27: unnecessary conversion
/Users/tamird/src/go/src/github.com/cockroachdb/cockroach/storage/engine/rocksdb.go:1039:8: unnecessary conversion
panic: illegal file offset

goroutine 1 [running]:
panic(0x250080, 0xc82fe25600)
    /Users/tamird/src/go1.6/src/runtime/panic.go:481 +0x3e6
go/token.(*File).Pos(0xc8320f0c60, 0x9b2f, 0xc83a281d08)
    /Users/tamird/src/go1.6/src/go/token/position.go:237 +0x78
main.print(0xc82accb2c0, 0x53, 0xc8320c1450)
    /Users/tamird/src/go/src/github.com/mdempsky/unconvert/unconvert.go:120 +0x3ef
main.main()
    /Users/tamird/src/go/src/github.com/mdempsky/unconvert/unconvert.go:220 +0x66b

Aside from the panic, all the reported issues appear to be false positives.

mdempsky commented 8 years ago

Yeah, those C.foo conversions look like false positives to me too.

tamird commented 8 years ago

Note that with #12 applied, I'm seeing https://github.com/mdempsky/unconvert/pull/12/files#diff-d45c7d690c87bd02c6ddd98b0dbd510fR113 fire - so it looks like maybe we're getting a real positive on the cgo-processed source which is not present in the original source.

mdempsky commented 8 years ago

unconvert github.com/cockroachdb/cockroach/... now runs without crashing. Will followup on improving error precision in separate issues (e.g., #16).