Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1234)

Issue 10411045: code review 10411045: go.tools/go/types: return indirection information from ... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 9 months ago by gri
Modified:
10 years, 9 months ago
Reviewers:
adonovan
CC:
adonovan, golang-dev
Visibility:
Public.

Description

go.tools/go/types: improved LookupFieldOrMethod, ast.Nodes for Scopes - LookupFieldOrMethod now computes if any indirection was found on the way to an embedded field/method: this is the only information required to determine if a result method is in the method set. - Scopes now provide a link to the ast.Node responsible for them. Also: - don't permit unsafe.Offsetof on method values - report ambiguities in field/method lookup errors - added some missing checks for anonymous fields - lots of new tests Fixes issue 5499.

Patch Set 1 #

Patch Set 2 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Patch Set 3 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Patch Set 4 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Patch Set 5 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Patch Set 6 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Patch Set 7 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Patch Set 8 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Patch Set 9 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Patch Set 10 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Patch Set 11 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Patch Set 12 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Total comments: 14

Patch Set 13 : diff -r f40ac11d013b https://code.google.com/p/go.tools #

Unified diffs Side-by-side diffs Delta from patch set Stats (+419 lines, -87 lines) Patch
M go/types/api.go View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M go/types/builtins.go View 1 2 3 4 5 1 chunk +11 lines, -6 lines 0 comments Download
M go/types/check.go View 1 2 3 4 5 6 7 8 9 1 chunk +6 lines, -0 lines 0 comments Download
M go/types/check_test.go View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M go/types/expr.go View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +56 lines, -8 lines 0 comments Download
M go/types/lookup.go View 1 2 3 4 5 6 7 8 9 10 11 12 10 chunks +66 lines, -31 lines 0 comments Download
M go/types/resolver.go View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -0 lines 0 comments Download
M go/types/scope.go View 1 2 3 4 5 6 7 8 9 2 chunks +10 lines, -1 line 0 comments Download
M go/types/sizes.go View 1 2 3 4 5 1 chunk +3 lines, -6 lines 0 comments Download
M go/types/stmt.go View 1 2 3 4 5 6 7 8 9 7 chunks +15 lines, -11 lines 0 comments Download
M go/types/testdata/builtins.src View 1 2 3 2 chunks +6 lines, -0 lines 0 comments Download
M go/types/testdata/decls0.src View 1 2 3 4 5 2 chunks +3 lines, -4 lines 0 comments Download
M go/types/testdata/decls2a.src View 1 1 chunk +1 line, -1 line 0 comments Download
M go/types/testdata/decls3.src View 1 2 3 4 5 6 7 8 9 10 11 12 8 chunks +39 lines, -13 lines 0 comments Download
M go/types/testdata/expr3.src View 1 2 3 4 5 2 chunks +8 lines, -5 lines 0 comments Download
A go/types/testdata/methodsets.src View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +190 lines, -0 lines 0 comments Download
M go/types/types.go View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 4
gri
Hello adonovan@google.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go.tools
10 years, 9 months ago (2013-06-20 22:36:03 UTC) #1
adonovan
LGTM https://codereview.appspot.com/10411045/diff/31001/go/types/check.go File go/types/check.go (right): https://codereview.appspot.com/10411045/diff/31001/go/types/check.go#newcode26 go/types/check.go:26: // TODO(gri) Decide if this should be a ...
10 years, 9 months ago (2013-06-21 13:54:26 UTC) #2
gri
https://codereview.appspot.com/10411045/diff/31001/go/types/check.go File go/types/check.go (right): https://codereview.appspot.com/10411045/diff/31001/go/types/check.go#newcode26 go/types/check.go:26: // TODO(gri) Decide if this should be a permanent ...
10 years, 9 months ago (2013-06-21 15:55:58 UTC) #3
gri
10 years, 9 months ago (2013-06-21 15:57:30 UTC) #4
*** Submitted as
https://code.google.com/p/go/source/detail?r=7a0800d10ab0&repo=tools ***

go.tools/go/types: improved LookupFieldOrMethod, ast.Nodes for Scopes

- LookupFieldOrMethod now computes if any indirection was found on the
  way to an embedded field/method: this is the only information required
  to determine if a result method is in the method set.

- Scopes now provide a link to the ast.Node responsible for them.

Also:
- don't permit unsafe.Offsetof on method values
- report ambiguities in field/method lookup errors
- added some missing checks for anonymous fields
- lots of new tests

Fixes issue 5499.

R=adonovan
CC=golang-dev
https://codereview.appspot.com/10411045
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b