From bdd0ff08db7abf07db29cd6dca98b5c1bc26ef26 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Thu, 22 Aug 2019 21:27:33 -0700 Subject: [PATCH] go/types: process each segment of delayed actions in FIFO order The stack of delayed actions is grown by pushing a new action on top with Checker.later. Checker.processDelayed processes all actions above a top watermark and then resets the stack to top. Until now, pushed actions above the watermark were processed in LIFO order. This change processes them in FIFO order, which seems more natural (if an action A was delayed before an action B, A should be processed before B for that stack segment). (With this change, Checker.later could be used instead of Checker.atEnd to postpone interface method type comparison and then the specific example in issue #33656 does type-check. However, in general we want interface method type comparisons to run after all interfaces are completed. With Checker.later we may still end up mixing interface completions and interface method type comparisons in ways leading to other errors for sufficiently convoluted code.) Also, move Checker.processDelayed from resolver.go to check.go. Change-Id: Id31254605e6944c490eab410553fff907630cc64 Reviewed-on: https://go-review.googlesource.com/c/go/+/191458 Reviewed-by: Matthew Dempsky --- src/go/types/check.go | 15 +++++++++++++++ src/go/types/resolver.go | 10 ---------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/go/types/check.go b/src/go/types/check.go index 7d58183911..eec33057de 100644 --- a/src/go/types/check.go +++ b/src/go/types/check.go @@ -269,6 +269,21 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) { return } +// processDelayed processes all delayed actions pushed after top. +func (check *Checker) processDelayed(top int) { + // If each delayed action pushes a new action, the + // stack will continue to grow during this loop. + // However, it is only processing functions (which + // are processed in a delayed fashion) that may + // add more actions (such as nested functions), so + // this is a sufficiently bounded process. + for i := top; i < len(check.delayed); i++ { + check.delayed[i]() // may append to check.delayed + } + assert(top <= len(check.delayed)) // stack must not have shrunk + check.delayed = check.delayed[:top] +} + func (check *Checker) processFinals() { n := len(check.finals) for _, f := range check.finals { diff --git a/src/go/types/resolver.go b/src/go/types/resolver.go index 417e4e79aa..93de63b059 100644 --- a/src/go/types/resolver.go +++ b/src/go/types/resolver.go @@ -607,16 +607,6 @@ func (a inSourceOrder) Len() int { return len(a) } func (a inSourceOrder) Less(i, j int) bool { return a[i].order() < a[j].order() } func (a inSourceOrder) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -// processDelayed processes all delayed actions pushed after top. -func (check *Checker) processDelayed(top int) { - for len(check.delayed) > top { - i := len(check.delayed) - 1 - f := check.delayed[i] - check.delayed = check.delayed[:i] - f() // may append to check.delayed - } -} - // unusedImports checks for unused imports. func (check *Checker) unusedImports() { // if function bodies are not checked, packages' uses are likely missing - don't check