code review 6974045: go/doc: don't synthesize code for examples that are not...

go/doc: don't synthesize code for examples that are not self-contained

Fixes issue 4309.

[adg, 2012-12-19 23:22:17]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[rsc, 2012-12-20 19:28:23]

It looks like the current code does:

1. Find top-level names in this file.
2. Reject examples using those names.
3. Ignore mentions of predefined identifiers like int.
4. Assume all other undefined names refer to imports.
5. Introduce those imports.

This seems backward to me. Among other things it does not handle uses of
top-level names declared elsewhere.

Instead of all this guessing, why not:

1. Look at import list to make list of packages (assume last element = package
name).
2. Make list of top-level names used by code.
3. Exclude predefined names (int)
4. Exclude imported packages (but remember that the import is needed).
5. Reject any other use of an unknown name.

Russ

[adg, 2012-12-20 19:48:25]

The second list seems incomplete. How does it determine which are top-level
names?

How about:

1. Walk the example function, noting:
 1a. names declared inside the function
 1b. otherwise resolved names
 1c. unresolved names
2. If anything in 1c, bail.
3. Exclude predefined identifiers from 1b
4. Exclude imported packages from 1b (but note them)
5. If anything remains in 1b, bail
6. Synthesize new file, import list, and main function.

?

[rsc, 2012-12-20 19:57:15]

> How about:
>
> 1. Walk the example function, noting:
>  1a. names declared inside the function
>  1b. otherwise resolved names
>  1c. unresolved names
> 2. If anything in 1c, bail.
> 3. Exclude predefined identifiers from 1b
> 4. Exclude imported packages from 1b (but note them)
> 5. If anything remains in 1b, bail
> 6. Synthesize new file, import list, and main function.

I see: I was assuming that id.Obj==nil for top-level names, but it's
only nil for top-level unresolved names. So you do need some way to
tell whether a resolved name points to a declared top-level name or
not. That's fine.
Note that if you are looking at a single file, predefined identifiers
like bool are in 1c (they have id.Obj==nil) not 1b. They are not
resolved because you need information from the other files to make the
final decision: if you have var int = 1 in some other file in the same
package, then 'int' refers to that.

Russ

[adg, 2012-12-20 20:00:27]

On 21 December 2012 06:57, Russ Cox <rsc@golang.org> wrote:

> That's fine.


So should I just stick with what I have, or rewrite a bit to get what I
proposed?

The one thing I haven't tried yet is looking for declarations. Shouldn't be
a big deal.

[rsc, 2012-12-20 20:04:50]

LGTM

It looks fine now that I know you have to handle those top-level resolved
names.

[adg, 2012-12-20 20:06:47]

*** Submitted as https://code.google.com/p/go/source/detail?r=41b82b6b2ccd ***

go/doc: don't synthesize code for examples that are not self-contained

Fixes issue 4309.

R=golang-dev, rsc
CC=golang-dev
https://codereview.appspot.com/6974045