Skip to content

Commit

Permalink
go/types, types2: remove non-test uses of Scope.LookupParent
Browse files Browse the repository at this point in the history
This moves the implementation of Scope.LookupParent into
environment.lookupScope where it encapsulates the use of
the current environment's position. At least in types2,
that position can be removed, because it is never set.

With this, the type checker doesn't rely on position
information anymore for looking up objects during type
checking.

LookupParent is still called from tests and some go/types
code.

Updates #69673.

Change-Id: I7159ba95b71cf33cc3b16058aa19327e166224b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/616337
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>
  • Loading branch information
griesemer authored and gopherbot committed Sep 27, 2024
1 parent 1646f72 commit 8031651
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 12 deletions.
21 changes: 19 additions & 2 deletions src/cmd/compile/internal/types2/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,26 @@ type environment struct {
hasCallOrRecv bool // set if an expression contains a function call or channel receive operation
}

// lookup looks up name in the current environment and returns the matching object, or nil.
// lookupScope looks up name in the current environment and if an object
// is found it returns the scope containing the object and the object.
// Otherwise it returns (nil, nil).
//
// Note that obj.Parent() may be different from the returned scope if the
// object was inserted into the scope and already had a parent at that
// time (see Scope.Insert). This can only happen for dot-imported objects
// whose parent is the scope of the package that exported them.
func (env *environment) lookupScope(name string) (*Scope, Object) {
for s := env.scope; s != nil; s = s.parent {
if obj := s.Lookup(name); obj != nil && (!env.pos.IsKnown() || cmpPos(obj.scopePos(), env.pos) <= 0) {
return s, obj
}
}
return nil, nil
}

// lookup is like lookupScope but it only returns the object (or nil).
func (env *environment) lookup(name string) Object {
_, obj := env.scope.LookupParent(name, env.pos)
_, obj := env.lookupScope(name)
return obj
}

Expand Down
7 changes: 6 additions & 1 deletion src/cmd/compile/internal/types2/scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (s *Scope) Lookup(name string) Object {
// Note that obj.Parent() may be different from the returned scope if the
// object was inserted into the scope and already had a parent at that
// time (see Insert). This can only happen for dot-imported objects
// whose scope is the scope of the package that exported them.
// whose parent is the scope of the package that exported them.
func (s *Scope) LookupParent(name string, pos syntax.Pos) (*Scope, Object) {
for ; s != nil; s = s.parent {
if obj := s.Lookup(name); obj != nil && (!pos.IsKnown() || cmpPos(obj.scopePos(), pos) <= 0) {
Expand All @@ -113,6 +113,11 @@ func (s *Scope) Insert(obj Object) Object {
return alt
}
s.insert(name, obj)
// TODO(gri) Can we always set the parent to s (or is there
// a need to keep the original parent or some race condition)?
// If we can, than we may not need environment.lookupScope
// which is only there so that we get the correct scope for
// marking "used" dot-imported packages.
if obj.Parent() == nil {
obj.setParent(s)
}
Expand Down
4 changes: 1 addition & 3 deletions src/cmd/compile/internal/types2/typexpr.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ func (check *Checker) ident(x *operand, e *syntax.Name, def *TypeName, wantType
x.mode = invalid
x.expr = e

// Note that we cannot use check.lookup here because the returned scope
// may be different from obj.Parent(). See also Scope.LookupParent doc.
scope, obj := check.scope.LookupParent(e.Value, check.pos)
scope, obj := check.lookupScope(e.Value)
switch obj {
case nil:
if e.Value == "_" {
Expand Down
21 changes: 19 additions & 2 deletions src/go/types/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,26 @@ type environment struct {
hasCallOrRecv bool // set if an expression contains a function call or channel receive operation
}

// lookup looks up name in the current environment and returns the matching object, or nil.
// lookupScope looks up name in the current environment and if an object
// is found it returns the scope containing the object and the object.
// Otherwise it returns (nil, nil).
//
// Note that obj.Parent() may be different from the returned scope if the
// object was inserted into the scope and already had a parent at that
// time (see Scope.Insert). This can only happen for dot-imported objects
// whose parent is the scope of the package that exported them.
func (env *environment) lookupScope(name string) (*Scope, Object) {
for s := env.scope; s != nil; s = s.parent {
if obj := s.Lookup(name); obj != nil && (!env.pos.IsValid() || cmpPos(obj.scopePos(), env.pos) <= 0) {
return s, obj
}
}
return nil, nil
}

// lookup is like lookupScope but it only returns the object (or nil).
func (env *environment) lookup(name string) Object {
_, obj := env.scope.LookupParent(name, env.pos)
_, obj := env.lookupScope(name)
return obj
}

Expand Down
7 changes: 6 additions & 1 deletion src/go/types/scope.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions src/go/types/typexpr.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *TypeName, wantType bo
x.mode = invalid
x.expr = e

// Note that we cannot use check.lookup here because the returned scope
// may be different from obj.Parent(). See also Scope.LookupParent doc.
scope, obj := check.scope.LookupParent(e.Name, check.pos)
scope, obj := check.lookupScope(e.Name)
switch obj {
case nil:
if e.Name == "_" {
Expand Down

0 comments on commit 8031651

Please sign in to comment.