go/packages: allow types loading without NeedDeps

[jirfag]

Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814,
fixes golang/go#31699, fixes golang/go#31930

[gopherbot]

This PR (HEAD: 2a85316) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/186337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186337.
After addressing review feedback, remember to publish your drafts!

[jirfag]

Looks like it fixes golang/go#31699 too,
and has conflicting code with golang/go#32814

/cc @matloob

[codeactual]

Thanks for this PR.

It also fixes golang/go#31930.

Now seeing run times and alloc_space profiles cut about in half for my use case, allowing me to query for types info and LoadSyntax but w/o the mandatory LoadSyntaxAll cost of parsing standard libraries, etc. that were not the target of the Load query.

[gopherbot]

This PR (HEAD: 3ae24b8) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/186337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: 979bdb7) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/186337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[jirfag]

I've resolved merge conflicts.
@matloob @ianthehat @stamblerre please, take a look on it. This pull request fixes 5 issues about serious performance regression after splitting Load* into more fine-grained Need*.

[gopherbot]

Message from Michael Matloob:

Patch Set 3:

(2 comments)

I agree we should fix the issue where we load everything from source, even if the user doesn't need it, but I don't want to change the meaning of the NeedX fields.

Here's my idea to fix this (which I think should require minimal changes to your CL): Have the loader keep a separate set of "implied" NeedX bits, called, say, "impliedMode". Then addDependingLoadModes can set impliedMode instead of modifying ld.Mode directly. Because the loader keeps the original mode, it can then clear the bits the same way it was doing before this change.

What do you think?

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186337.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

This PR (HEAD: 08e470e) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/186337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: 3540c02) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/186337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Denis Isaev:

Patch Set 5:

Thank you for your comments! I've removed information about implied modes from Need* comments. And I've created requestedMode to save actually requested mode. Adding impliedMode will be much more difficult because:
a. We need to change the interface of drivers: goListDriver accepts Config as an argument and uses Mode from it. Or we need to add local field impliedMode into Config: I think it will add complexity for users of go/packages.
b. We need to change ~40 places in code Mode -> impliedMode.
c. It will be easy to use Mode instead of impliedMode: cost of supporting will be higher.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186337.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Michael Matloob:

Patch Set 5:

(2 comments)

Patch Set 5:

Thank you for your comments! I've removed information about implied modes from Need* comments. And I've created requestedMode to save actually requested mode. Adding impliedMode will be much more difficult because:
a. We need to change the interface of drivers: goListDriver accepts Config as an argument and uses Mode from it. Or we need to add local field impliedMode into Config: I think it will add complexity for users of go/packages.
b. We need to change ~40 places in code Mode -> impliedMode.
c. It will be easy to use Mode instead of impliedMode: cost of supporting will be higher.

Sounds good! just a couple of comments about the impliedMode function

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186337.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Michael Matloob:

Patch Set 5:

(2 comments)

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186337.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

This PR (HEAD: f015199) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/186337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Denis Isaev:

Patch Set 6:

(3 comments)

thank you for the review, I've fixed all comments, but it required me to do a lot of changes.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186337.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Michael Matloob:

Patch Set 6:

(1 comment)

Hi, sorry this review is taking so long.

Could you revert the changes you made in your most recent patch set (except for making impliedMode a function)? I'll submit that change and we can work on the rest in a separate review.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186337.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

This PR (HEAD: 2e3a46e) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/tools/+/186337 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Denis Isaev:

Patch Set 6:

ok, I've reverted the last change and left only impliedLoadMode function refactoring. Please, take a look at it again.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186337.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Michael Matloob:

Patch Set 7: Code-Review+2

(1 comment)

one more small thing

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186337.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

Message from Denis Isaev:

Patch Set 7:

(1 comment)

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/186337.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

This PR is being closed because golang.org/cl/186337 has been merged.