From 25775e59acb784aba9401400f89429ba69e7a0dd Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 23 Jun 2020 22:18:23 -0400 Subject: [PATCH] internal/memoize: add an error return to (*handle).Get Fixes golang/go#36004 Change-Id: I8da7c21eaa9cf6ffac12aabdd6803d06781cef32 Reviewed-on: https://go-review.googlesource.com/c/tools/+/239564 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/cache/analysis.go | 10 +++++----- internal/lsp/cache/cache.go | 6 +++--- internal/lsp/cache/check.go | 6 +++--- internal/lsp/cache/mod.go | 16 ++++++++-------- internal/lsp/cache/mod_tidy.go | 6 +++--- internal/lsp/cache/parse.go | 5 ++++- internal/lsp/cache/view.go | 6 +++--- internal/memoize/memoize.go | 16 ++++++++-------- internal/memoize/memoize_test.go | 7 +++++-- 9 files changed, 42 insertions(+), 36 deletions(-) diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go index 35336a763e..ba5fb39e6c 100644 --- a/internal/lsp/cache/analysis.go +++ b/internal/lsp/cache/analysis.go @@ -152,9 +152,9 @@ func (s *snapshot) actionHandle(ctx context.Context, id packageID, a *analysis.A } func (act *actionHandle) analyze(ctx context.Context) ([]*source.Error, interface{}, error) { - v := act.handle.Get(ctx) + v, err := act.handle.Get(ctx) if v == nil { - return nil, nil, ctx.Err() + return nil, nil, err } data, ok := v.(*actionData) if !ok { @@ -182,9 +182,9 @@ func execAll(ctx context.Context, actions []*actionHandle) (map[*actionHandle]*a for _, act := range actions { act := act g.Go(func() error { - v := act.handle.Get(ctx) - if v == nil { - return errors.Errorf("no analyses for %s", act.pkg.ID()) + v, err := act.handle.Get(ctx) + if err != nil { + return err } data, ok := v.(*actionData) if !ok { diff --git a/internal/lsp/cache/cache.go b/internal/lsp/cache/cache.go index 8a72479d0f..9ae32958ad 100644 --- a/internal/lsp/cache/cache.go +++ b/internal/lsp/cache/cache.go @@ -72,9 +72,9 @@ func (c *Cache) getFile(ctx context.Context, uri span.URI) (*fileHandle, error) h := c.store.Bind(key, func(ctx context.Context) interface{} { return readFile(ctx, uri, modTime) }) - v := h.Get(ctx) - if v == nil { - return nil, ctx.Err() + v, err := h.Get(ctx) + if err != nil { + return nil, err } return v.(*fileHandle), nil } diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 5c7d5f1e7f..3ab5b3f976 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -190,9 +190,9 @@ func (ph *packageHandle) Check(ctx context.Context) (source.Package, error) { } func (ph *packageHandle) check(ctx context.Context) (*pkg, error) { - v := ph.handle.Get(ctx) - if v == nil { - return nil, ctx.Err() + v, err := ph.handle.Get(ctx) + if err != nil { + return nil, err } data := v.(*packageData) return data.pkg, data.err diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go index bcd04f5a13..c0f98f998e 100644 --- a/internal/lsp/cache/mod.go +++ b/internal/lsp/cache/mod.go @@ -55,9 +55,9 @@ func (mh *parseModHandle) Sum() source.FileHandle { } func (mh *parseModHandle) Parse(ctx context.Context) (*modfile.File, *protocol.ColumnMapper, []source.Error, error) { - v := mh.handle.Get(ctx) - if v == nil { - return nil, nil, nil, ctx.Err() + v, err := mh.handle.Get(ctx) + if err != nil { + return nil, nil, nil, err } data := v.(*parseModData) return data.parsed, data.m, data.parseErrors, data.err @@ -200,9 +200,9 @@ type modWhyData struct { } func (mwh *modWhyHandle) Why(ctx context.Context) (map[string]string, error) { - v := mwh.handle.Get(ctx) - if v == nil { - return nil, ctx.Err() + v, err := mwh.handle.Get(ctx) + if err != nil { + return nil, err } data := v.(*modWhyData) return data.why, data.err @@ -287,9 +287,9 @@ type modUpgradeData struct { } func (muh *modUpgradeHandle) Upgrades(ctx context.Context) (map[string]string, error) { - v := muh.handle.Get(ctx) + v, err := muh.handle.Get(ctx) if v == nil { - return nil, ctx.Err() + return nil, err } data := v.(*modUpgradeData) return data.upgrades, data.err diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go index 485b16bddb..d00f36fabb 100644 --- a/internal/lsp/cache/mod_tidy.go +++ b/internal/lsp/cache/mod_tidy.go @@ -49,9 +49,9 @@ type modTidyData struct { } func (mth *modTidyHandle) Tidy(ctx context.Context) (map[string]*modfile.Require, []source.Error, error) { - v := mth.handle.Get(ctx) - if v == nil { - return nil, nil, ctx.Err() + v, err := mth.handle.Get(ctx) + if err != nil { + return nil, nil, err } data := v.(*modTidyData) return data.missingDeps, data.diagnostics, data.err diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go index 46aea457b9..a3b1427a88 100644 --- a/internal/lsp/cache/parse.go +++ b/internal/lsp/cache/parse.go @@ -91,7 +91,10 @@ func (pgh *parseGoHandle) Parse(ctx context.Context) (*ast.File, []byte, *protoc } func (pgh *parseGoHandle) parse(ctx context.Context) (*parseGoData, error) { - v := pgh.handle.Get(ctx) + v, err := pgh.handle.Get(ctx) + if err != nil { + return nil, err + } data, ok := v.(*parseGoData) if !ok { return nil, errors.Errorf("no parsed file for %s", pgh.File().URI()) diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index f04a988d3b..8faab5b450 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -280,9 +280,9 @@ func (v *View) BuiltinPackage(ctx context.Context) (source.BuiltinPackage, error if v.builtin == nil { return nil, errors.Errorf("no builtin package for view %s", v.name) } - data := v.builtin.handle.Get(ctx) - if ctx.Err() != nil { - return nil, ctx.Err() + data, err := v.builtin.handle.Get(ctx) + if err != nil { + return nil, err } if data == nil { return nil, errors.Errorf("unexpected nil builtin package") diff --git a/internal/memoize/memoize.go b/internal/memoize/memoize.go index 30f4c0c4be..595d438252 100644 --- a/internal/memoize/memoize.go +++ b/internal/memoize/memoize.go @@ -203,9 +203,9 @@ func (h *Handle) Cached() interface{} { // If the value is not yet ready, the underlying function will be invoked. // This activates the handle, and it will remember the value for as long as it exists. // If ctx is cancelled, Get returns nil. -func (h *Handle) Get(ctx context.Context) interface{} { +func (h *Handle) Get(ctx context.Context) (interface{}, error) { if ctx.Err() != nil { - return nil + return nil, ctx.Err() } h.mu.Lock() switch h.state { @@ -215,14 +215,14 @@ func (h *Handle) Get(ctx context.Context) interface{} { return h.wait(ctx) case stateCompleted: defer h.mu.Unlock() - return h.value + return h.value, nil default: panic("unknown state") } } // run starts h.function and returns the result. h.mu must be locked. -func (h *Handle) run(ctx context.Context) interface{} { +func (h *Handle) run(ctx context.Context) (interface{}, error) { childCtx, cancel := context.WithCancel(xcontext.Detach(ctx)) h.cancel = cancel h.state = stateRunning @@ -258,7 +258,7 @@ func (h *Handle) run(ctx context.Context) interface{} { } // wait waits for the value to be computed, or ctx to be cancelled. h.mu must be locked. -func (h *Handle) wait(ctx context.Context) interface{} { +func (h *Handle) wait(ctx context.Context) (interface{}, error) { h.waiters++ done := h.done h.mu.Unlock() @@ -268,9 +268,9 @@ func (h *Handle) wait(ctx context.Context) interface{} { h.mu.Lock() defer h.mu.Unlock() if h.state == stateCompleted { - return h.value + return h.value, nil } - return nil + return nil, nil case <-ctx.Done(): h.mu.Lock() defer h.mu.Unlock() @@ -282,7 +282,7 @@ func (h *Handle) wait(ctx context.Context) interface{} { h.done = nil h.cancel = nil } - return nil + return nil, ctx.Err() } } diff --git a/internal/memoize/memoize_test.go b/internal/memoize/memoize_test.go index 1ac05130eb..305b594db9 100644 --- a/internal/memoize/memoize_test.go +++ b/internal/memoize/memoize_test.go @@ -217,8 +217,11 @@ func joinValues(ctx context.Context, s *memoize.Store, name string, keys ...stri fmt.Fprintf(w, "start %v\n", name) value := "" for _, key := range keys { - v := asValue(s.Bind(key, generate(s, key)).Get(ctx)) - if v == nil { + i, err := s.Bind(key, generate(s, key)).Get(ctx) + if err != nil { + return &stringOrError{err: err} + } + if v := asValue(i); v == nil { value = value + " " } else if v.err != nil { value = value + " !" + v.err.Error()