Replace use of reflect.MethodByName with real interface to allow Go dead-code elimination

[mappu]

Hi,

In current versions of the Go compiler, any use of reflect.MethodByName halts the entire dead-code elimination pass:

https://go.dev/src/cmd/link/internal/ld/deadcode.go#L293

This is causing many Go binaries with go-sqlite3 to be much larger than they need to be.

reflect.MethodByName is used in go-sqlite3 for custom aggregation functions and calling Step() and Call() on them:

https://github.com/mattn/go-sqlite3/blob/master/sqlite3.go#L453

From a quick reading of this, it seems like it might be possible to

• cast to an interface { Step(); Call(); } and avoid the use of reflect.MethodByName; or
• pass in step/call callbacks, similar to how the constructor is handled; or
• support a build tag like noreflect or noaggregator to disable this functionality entirely if it's unused

[rittneje]

cast to an interface { Step(); Call(); } and avoid the use of reflect.MethodByName; or

This will not work because the signature of Step (specifically the argument types) is variable.

[falco467]

Two options: Is there a finite number of possibilities for argument-types in the step function? Then one could define multiple interfaces and use a type-select to call the right one. Otherwise could the Method with the reflect-call be moved into its own method/package, which could be eliminated by the dead-code analysis, if the whole package with this call is not needed ?

[rittneje]

Is there a finite number of possibilities for argument-types in the step function?

No, the Step function may contain an arbitrary number of parameters.

In addition, currently RegisterAggregator will allow (as in not return an error) Step methods where one of the arguments is a typedef of a supported type. For example:

type Foo string
func (*myAggregator) Step(f Foo){ ... }

However, if you actually try to use such an aggregator in a query, it will panic. Either we have to start disallowing such typedefs within RegisterAggregator (which could be considered a breaking change), or the code needs to be fixed to handle them properly, in which case even the parameter types will be arbitrary.

Otherwise could the Method with the reflect-call be moved into its own method/package

This would also be a breaking change.

Unfortunately, other than being able to remove the method in question via a build tag, I can't think of any way to address this without incurring some sort of breaking change.