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.

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

go/analysis: latent bug in unitchecker fact registration #57131

Open
adonovan opened this issue Dec 7, 2022 · 0 comments
Open

go/analysis: latent bug in unitchecker fact registration #57131

adonovan opened this issue Dec 7, 2022 · 0 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Dec 7, 2022

While reading code in unitchecker I noticed that the algorithm for Fact registration appears to be backwards (my bad), as noted by the TODO comment which I added in CL 443099. This issue is a reminder to fix it.

	// Register fact types with gob.
	// In VetxOnly mode, analyzers are only for their facts,
	// so we can skip any analysis that neither produces facts
	// nor depends on any analysis that produces facts.
	//
	// TODO(adonovan): fix: the command (and logic!) here are backwards.
	// It should say "...nor is required by any...".
	//
	// Also build a map to hold working state and result.
	type action struct {
		once        sync.Once
		result      interface{}
		err         error
		usesFacts   bool // (transitively uses)
		diagnostics []analysis.Diagnostic
	}
	actions := make(map[*analysis.Analyzer]*action)
	var registerFacts func(a *analysis.Analyzer) bool
	registerFacts = func(a *analysis.Analyzer) bool {
		act, ok := actions[a]
		if !ok {
			act = new(action)
			var usesFacts bool
			for _, f := range a.FactTypes {
				usesFacts = true
				gob.Register(f)
			}
			for _, req := range a.Requires {
				if registerFacts(req) {
					usesFacts = true
				}
			}
			act.usesFacts = usesFacts
			actions[a] = act
		}
		return act.usesFacts
	}
@prattmic prattmic added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 7, 2022
@seankhliao seankhliao added this to the Backlog milestone Jan 20, 2023
@adonovan adonovan added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants