x/tools/go/analysis: specify that all facts are encoded in one gulp, preserving topology

[adonovan]

The go/analysis Fact mechanism allows an analysis to associate information with the symbols defined by one package so that it may be retrieved during analysis of another package that imports these symbols. Facts must be pointers to gob-serializable data structures. In many analyses, facts naturally correspond to nodes in some domain-specific graph representation, so that two or more facts conceptually share a large common data structure such as a call graph. It would be a pity if the encoding of such facts caused the representation to explode by a linear factor.

The current specification of the analysis API permits a driver implementation to encode a collection of facts in a single gob operation, so that any shared portions are encoded exactly once and decoding preserves the original topology. The existing drivers in x/tools use the facts helper package and thus already function in this way; this has been verified experimentally.

I propose to change the specification to state that this behavior is required by all drivers. In the meantime, any existing third-party drivers that do not conform may run slower and use more memory for new analysis passes that assume this requirement, but should otherwise compute the correct answer.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

I propose to change the specification to state that this behavior is required by all drivers.

Are there even any other drivers?
In any event, this sounds fine to me (as one of the designers of this API).

Does anyone object to this proposal?

[nightlyone]

@adonovan / @rsc Did you crosscheck this with the authors (e.g. @ldez ) of golangci-lint?

I would like to understand whether that has any effects on the largest linter integration project of the go ecosystem.

[adonovan]

Are there even any other drivers?

I'm aware of at least two: golangci-lint, lintcmd.

Did you crosscheck this with the authors (e.g. @ldez ) of golangci-lint?
I would like to understand whether that has any effects on the largest linter integration project of the go ecosystem.

Good idea; I had not.

golangci-lint decodes a slice of facts all at once but encodes them one at a time, so it would need to be updated.

lintcmd decodes facts one at a time, so it too would need to be updated.

[rsc]

Because:

In the meantime, any existing third-party drivers that do not conform may run slower and use more memory for new analysis passes that assume this requirement, but should otherwise compute the correct answer.

This seems fine to do even though there are a couple drivers. They can update or not as necessary, but nothing will break.

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group