cmd/go: add vet check for json/xml omitempty struct tags on non-basic types #63541
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n other repos TestImportedTypes was assuming that go/internal/gcimporter declares a FindPkg function with a particular signature. However, since go/internal/gcimporter is internal, it is not subject to Go 1 compatibility and its API is subject to change. I intend to change it in CL 507360. For golang#61064. Change-Id: I994f5b967ce9c57d07ce1fe8b6ce900176848efa Reviewed-on: https://go-review.googlesource.com/c/tools/+/511758 Auto-Submit: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Run-TryBot: Bryan Mills <[email protected]>
When analysis takes a long time, it can be a poor experience for our users. Address this in a number of ways: - Limit parallelism of analysis, so that it doesn't consume all available memory. - Add progress reporting if analysis takes longer than 5s, for better visibility and to allow cancellation. In case this is annoying to users, add a setting to disable this feature. For golang#61178 Change-Id: I15e05f39c606ff7a3fecee672918ee3c4a640fbc Reviewed-on: https://go-review.googlesource.com/c/tools/+/511215 gopls-CI: kokoro <[email protected]> Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
Gopls can rely on a deterministic ordering of methods, and therefore does not need to sort in the objectpath algorithm, which turns out to be enormously expensive on certain repositories (see golang#61178). Add hooks to skip method sorting during encode/decode, and use this for analysis facts. This CL is intentionally surgical, to avoid unintended side-effects for this change in behavior. Notably, other uses of objectpath are unaffected. For golang#61178 Change-Id: If915dab45b837e23ae5b841e3a9367aa0b53df89 Reviewed-on: https://go-review.googlesource.com/c/tools/+/511216 Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
Change-Id: Iaaa21c431f20ebd708713884583cfbe59a82d64b Reviewed-on: https://go-review.googlesource.com/c/tools/+/511716 Run-TryBot: Alan Donovan <[email protected]> Auto-Submit: Alan Donovan <[email protected]> Reviewed-by: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]>
Our arbitrary choice of caching 200 recent files has proven to be problematic when editing very large packages (golang#61207). Fix this by switching to time-based eviction, but with a minimum cache size to optimize the case where editing is resumed after a while. This caused an increase in high-water mark during initial workspace load, but that is mitigated by avoiding the parse cache when type checking for import (i.e. non-workspace files): such parsed files are almost never cache hits as they are only ever parsed once, and would instead benefit more from avoiding ParseComments. Move the ownership of the parseCache to the cache.Session (and pass it to each View) to make its lifecycle clearer and avoid passing it around to each snapshot. For golang#61207 Change-Id: I357d8b1fa36eabb516dbb7147266df0e5153ac11 Reviewed-on: https://go-review.googlesource.com/c/tools/+/511337 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]>
When working in a package we must repeatedly re-build package handles, which requires parsing with purged func bodies. Although purging func bodies leads to faster parsing, it is still expensive enough to warrant caching. Move the 'purgeFuncBodies' field into the parse cache. For golang#61207 Change-Id: I90575e5b6be7181743e8376c24312115a1029188 Reviewed-on: https://go-review.googlesource.com/c/tools/+/503440 gopls-CI: kokoro <[email protected]> Reviewed-by: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Robert Findley <[email protected]>
Analysis has far too many threads running at once, resulting in cache thrashing. Reduce parallelism of runCached, cache update, and parsing. (The first of these is logic borrowed from CL 511215.) Change-Id: I3ec654bd49965b3cb9e0f06cc7f49dcd01093778 Reviewed-on: https://go-review.googlesource.com/c/tools/+/511755 Reviewed-by: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Auto-Submit: Alan Donovan <[email protected]> Run-TryBot: Alan Donovan <[email protected]>
This change tracks the number of unfinished predecessors in the batch of analysis nodes, and when it falls to zero, it clears the analysisSummary.Actions field, which may hold large structures such as SSA whose lifetime we should not prolong unnecessarily. Change-Id: I5bdd6c1e473e4556e5bc7d86a9f4d62488f88199 Reviewed-on: https://go-review.googlesource.com/c/tools/+/511735 Auto-Submit: Alan Donovan <[email protected]> Reviewed-by: Robert Findley <[email protected]> Run-TryBot: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
Setting maxAge=0 should degenerate the parse cache to evict immediately (necessary to avoid flakiness in tests on platforms with coarse clock granularity). Fixes golang#61550 Change-Id: I6cb03cfb670faa6c6995fd3c4c5a98dd1dadb344 Reviewed-on: https://go-review.googlesource.com/c/tools/+/512635 Reviewed-by: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]>
This change causes Set.Export to throw away facts that it itself imported, unless they are about fields or methods, or package-level types. All other facts related to objects that are only obtained via direct imports. Package facts are not reexported. In effect this approximates the pruning of "deep" export data, in contrast to no pruning at all (!!). There's more we could here even just to get to parity with "deep" export data pruning. Change-Id: I7377ebf86fb355121a6f2f0e0f30486f553296c1 Reviewed-on: https://go-review.googlesource.com/c/tools/+/511336 Reviewed-by: Robert Findley <[email protected]> Run-TryBot: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]>
Add missing benchmarks for codeactions. For golang#61508 Change-Id: I260fdae1c70c2e2291b7469cb7240d3d68cf1ded Reviewed-on: https://go-review.googlesource.com/c/tools/+/512495 Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alan Donovan <[email protected]> gopls-CI: kokoro <[email protected]>
We were memoizing active packages only after a call to TypeCheck. This means that we may duplicate work if diagnostics, references, or implements requests execute before the first call to TypeCheck for an open package. Fix this by pushing the memoization logic down into forEachPackage. This uncovered a bug in go/types that may have already been affecting users: golang#61561. This bug is consistent with user reports on slack of strange errors related to missing methods. Avoid this bug in most cases by ensuring that all instantiated interfaces are completed immediately after type checking. However, this doesn't completely avoid the bug as it may not complete parameterized interface literals elsewhere in the type definition. For example, the following definition is still susceptible to the bug: type T[P any] interface { m() interface{ n(P) } } For golang#60926 Fixes golang#61005 Change-Id: I324cd13bac7c17b1eb5642973157bdbfb2368650 Reviewed-on: https://go-review.googlesource.com/c/tools/+/512636 Run-TryBot: Robert Findley <[email protected]> Reviewed-by: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]>
As noted in an outstanding TODO, there's no reason to re-compute diagnostics during code actions. This may have been cheap in the previous gopls architecture, but still complicated the code. With the new architecture it has significant cost, especially when running analysis with a different set of analyzers. Fix this by instead unbundling serialized code actions, or failing that by matching code action context with previously published diagnostics stored on the server. After doing so, what remained was the additional "diagnostics" produced by convenience analyzers. These are (rather pragmatically) refactored to be computed from typed syntax alone, outside of the analysis framework. Incidentally, this fixes the long-standing golang#51478. The analyzers are left as thin shells, so that existing analyzer configuration will continue to work for the short term. Longer term, we should design a new mechanism for parameterizing refactorings and deprecate the old analyzer settings. Make some additional cleanup along the way, and some other relatively superficial changes to get tests to pass. Fixes golang#51478 Fixes golang#61508 Change-Id: I0218d4fc15a6fdb5f2fd5c0560b9d9afa079b84d Reviewed-on: https://go-review.googlesource.com/c/tools/+/511995 gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Run-TryBot: Robert Findley <[email protected]>
When importing, it is also possible to produce an incomplete interface instance, which can be racy (as reproduced in the included test). Partially avoid this by completing interface underlyings of instances. For golang#61561 Change-Id: I8ca2bdc2d03fa1a46179bbd8596d7ef9baf51600 Reviewed-on: https://go-review.googlesource.com/c/tools/+/512955 gopls-CI: kokoro <[email protected]> Run-TryBot: Robert Findley <[email protected]> Reviewed-by: Alan Donovan <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
This utilizes the code copied from honnef.co/tools/staticcheck. Unlike staticcheck.CheckDeprecated that assumes analyzers with `-go` flag specifying the target go version and utilizes structured knowledge of stdlib deprecated symbols, this analyzer depends on the facts and surfaces the deprecation notices as written in the comments. We also simplified implementation during review. Gopls turns this analyzer's reports to Hint/Deprecated diagnostics. Editors like VS Code strike out the marked symbols but don't surface them in the PROBLEMS panel. Fixes golang#40447 Change-Id: I7ac04c6008587e3c71689dec5cb4ec06523b67f3 Reviewed-on: https://go-review.googlesource.com/c/tools/+/508508 Reviewed-by: Alan Donovan <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
Fix an embarassing oversight of CL 512636: the check for active packages was failing deterministically. Also update the definition benchmark to instrument CPU profiling. Change-Id: I544da1d9395ed5eadf51338a29787e8404848347 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513096 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]> Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> Auto-Submit: Robert Findley <[email protected]>
Following CL 510095, we no longer await loading in resolveImportGraph. However, we were still acquiring the snapshot metadata once, before requesting MetadataForFile. This means that the metadata used to build package handles had more information than the metadata used to invalidate volatile package handles, causing some deps not to be present in the handles map. While at it, change cache.Package to hold only Metadata, not packageHandle. Going forward, packageHandles should be scoped more narrowly to the typecheckBatch logic. Fixes golang#61512 Change-Id: I32fc248b3b1329e071caf0cb304f55a6a29f8c49 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513095 Reviewed-by: Hyang-Ah Hana Kim <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]> Run-TryBot: Robert Findley <[email protected]>
Change-Id: I4c5169661f9f445a2d4920786a85df70797ca24a Reviewed-on: https://go-review.googlesource.com/c/tools/+/512835 Reviewed-by: Ian Lance Taylor <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]> Run-TryBot: shuang cui <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Auto-Submit: Ian Lance Taylor <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> gopls-CI: kokoro <[email protected]>
As noted recently in gopls/internal/lsp/protocol/context.go, log messages are delivered asynchronously to other LSP events. This means that any test relying on sequencing of log messages with respect to other operations is fundamentally incorrect. Searching for such tests turned up several known flakes. Fixes golang#61521 Fixes golang#51549 Fixes golang#51772 Change-Id: I4cb5031140e34e1c816b221a90d5ffa4055ad76c Reviewed-on: https://go-review.googlesource.com/c/tools/+/513097 Run-TryBot: Robert Findley <[email protected]> Reviewed-by: Alan Donovan <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
Another facepalm. Discovered during manual testing of the prerelease. Change-Id: I5fdb173a68ad8659d773259bcc737b4f708d23ab Reviewed-on: https://go-review.googlesource.com/c/tools/+/512957 gopls-CI: kokoro <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
Partially revert CL 512636, putting back the fast path to fetch the active package. This fixes a "regression" in certain fast benchmarks, such as Definition, Hover, or References in a local package. The numbers are still small either way, but may matter in very large repos. Change-Id: Id850eaa7a2599d9fb6ad042e762b59b3d5220cf1 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513101 Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
VS Code exposes only one line for progress reports; as pointed out in golang#61352, multi-line reports cause a broken UI. Fixes golang#61352 Change-Id: I0b5864f9a0dbb0bd9d191075b6c1cf0c7285f4bc Reviewed-on: https://go-review.googlesource.com/c/tools/+/513735 Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]> gopls-CI: kokoro <[email protected]>
Give the embeddirective source.Analyzer a Fix that can add missing "embed" imports. Since not all reports from the analyzer is this diagnostic, give source.Analyzer an IsFixable method to filter diagnostics. Updates golang#50262 Change-Id: I24abdd2d1a88fc5cabd3197ef438c4da041a6a9c Reviewed-on: https://go-review.googlesource.com/c/tools/+/509056 Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Robert Findley <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Fix an oversight in the satisfaction check: composite lits may indeed have type parameter type, and therefore we must consider their core type. Fixes golang#61614 Change-Id: I2119ba308816d02742d8e790f8cd00c4d862e789 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513775 Reviewed-by: Hyang-Ah Hana Kim <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Robert Findley <[email protected]>
Bring the analyzer documentation up to speed. Include the recently added feature to check the declaration following the directive. Also fix typo in package comment. Updates golang#50262 Change-Id: I0e7a8de0ba10cd414251afe1e9c65ded2090f408 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513895 Reviewed-by: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Robert Findley <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]> gopls-CI: kokoro <[email protected]>
Type parameters were incorrectly being treated as exported for the purpose of renaming checks, and imported type parameters do not have the correct scope information for rename checks to work. Fixes golang#61635 Change-Id: I4261c5e7b3622affbeb81a96ea4f510a5b9e4fe3 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513916 gopls-CI: kokoro <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]> Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
Type names of named types are always canonical. The existing logic to canonicalize them was ineffective, and had the side effect of breaking aliases. Also add more tests of generic name renaming. Fixes golang#61625 Change-Id: I318a758176aea6712da16dde29394ffb08bdf715 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513917 Reviewed-by: Hyang-Ah Hana Kim <[email protected]> Run-TryBot: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]>
This logic was dropped in CL 464902, but is necessary for correctly resolving references to variables. Fixes golang#61618 Change-Id: Iebe46339e5f6ac0230f7376e742d786bae2a7438 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513780 Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Make a copy of cmd/internal/edit for use in this repo. This copy is taken from rsc.io/edit which is the cleaned up external version of cmd/internal/edit. (We don't want an rsc.io dependency in golang.org/x either.) Change-Id: I9d8ed5d58f61f14e032c613af711fafa050c7f3e Reviewed-on: https://go-review.googlesource.com/c/tools/+/513736 TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]> Auto-Submit: Russ Cox <[email protected]> Run-TryBot: Russ Cox <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
This is an experimental command that perhaps would become "go new" once we have more experience with it. We want to enable people to experiment with it and write their own templates and see how it works, and for that we need to put it in a place where it's reasonable to ask users to fetch it from. That place is golang.org/x/tools/cmd/gonew. There is an earlier copy in rsc.io/tmp/gonew, but that isn't the right place for end users to be fetching something to try. Once the tool is checked in I intend to start a GitHub discussion asking for feedback and suggestions about what is missing. I hope we will be able to identify core functionality that handles a large fraction of use cases. I've been using the earlier version myself for a while, and I've found it very convenient even in other contexts, like I want the code for a given module and don't want to go look up its Git repo and so on: go new rsc.io/[email protected] cd quote Change-Id: Ifc27cbd5d87ded89bc707b087b3f08fa70b1ef07 Reviewed-on: https://go-review.googlesource.com/c/tools/+/513737 gopls-CI: kokoro <[email protected]> Run-TryBot: Russ Cox <[email protected]> Auto-Submit: Russ Cox <[email protected]> Reviewed-by: Robert Findley <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
It was expensive, and unnecessary if we can rely on go/types and export data preserving source order (we can). Fixes golang#61443 Change-Id: I28d93c35f89eb751991c9d25aeb1c1904ba7b546 Reviewed-on: https://go-review.googlesource.com/c/tools/+/534139 Reviewed-by: Robert Findley <[email protected]> Auto-Submit: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
Change-Id: Ib1b0a228b47f1083b87285717d86ca111e62827e Reviewed-on: https://go-review.googlesource.com/c/tools/+/532276 Auto-Submit: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]>
Plus, a test. Change-Id: I9d3f4729c1b8da51d771442d9c3f5909f608591e Reviewed-on: https://go-review.googlesource.com/c/tools/+/534895 Reviewed-by: Robert Findley <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Alan Donovan <[email protected]>
These settings deeply affect gopls' behavior, and can't work well when set to a non-default value. More rationale is provided in the attached issues. The features controlled by these settings are not yet removed. For the v0.14.0 release, we will simply warn users who have them set. Fixes golang#57514 Fixes golang#61970 Change-Id: Ica631dcb8980eeeace9ceaec159218e6306831ae Reviewed-on: https://go-review.googlesource.com/c/tools/+/534919 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
|
This PR (HEAD: a63a669) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/535416. Important tips:
|
|
Message from Gopher Robot: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/535416. |
andig
force-pushed
the
vet/basic-types
branch
from
a63a669 to
6f00f52
Compare
|
This PR (HEAD: 6f00f52) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/535416. Important tips:
|
|
Message from Andreas Goetz: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/535416. |
|
Message from Andreas Goetz: Patch Set 1: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/535416. |
|
This PR (HEAD: 1d0e03a) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/535416. Important tips:
|
|
Message from Ian Lance Taylor: Patch Set 4: Commit-Queue+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/535416. |
|
Message from Go LUCI: Patch Set 4: Dry run: CV is trying the patch. Bot data: {"action":"start","triggered_at":"2023-10-14T14:07:57Z","revision":"c45e92a095aa5f5d6a63329f45d2c0dfd60ff637"} Please don’t reply on this GitHub thread. Visit golang.org/cl/535416. |
|
This PR (HEAD: 5846d37) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/535416. Important tips:
|
|
Message from Go LUCI: Patch Set 4: LUCI-TryBot-Result-1 Please don’t reply on this GitHub thread. Visit golang.org/cl/535416. |
|
Message from t hepudds: Patch Set 6: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/535416. |
|
Message from Tim King: Patch Set 6: Hold+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/535416. |
|
Message from Andreas Goetz: Patch Set 6: (3 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/535416. |
Previously, literalization, the strategy of last resort,
didn't make use of a binding decl even when one was available.
But a binding decl can make literalization more readable
as it puts the arguments before the body, their natural place,
which is important especially when the body is longer.
func(params) { body } (args)
=>
func() { var params = args; body }()
We don't yet attempt to do this if any named result is
referenced, because the binding decl would duplicate it;
teasing apart the params and results of the binding
decl is left for future work.
Plus tests.
Change-Id: I51da3016157c1531c2d57962c4bddb0203ac0946
Reviewed-on: https://go-review.googlesource.com/c/tools/+/535456
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
Use the new inlining technology to implement a first change-signature refactoring, by rewriting the declaration to be a trivial wrapper around a declaration with modified signature, inlining the wrapper, and then performing string substitution to replace references to the synthetic delegate. This demonstrates the power of inlining, which can be seen as a more general tool for rewriting source code: express old code as new code, recognize instances of old code (in this case, calls), and inline. However, this CL was still rather difficult, primarily because of (1) the problem of manipulating syntax without breaking formatting and comments, and (2) the problem of composing multiple refactoring operations, which in general requires iterative type checking. Neither of those difficulties have general solutions: any form of nontrivial AST manipulation tends to result in an unacceptable movement of comments, and iterative type checking is difficult because it may involve an arbitrarily modified package graph, and because it is difficult to correlate references in the previous version of the package with references in the new version of the package. But in the case of removing a parameter, these problems are solvable: We can avoid most AST mangling by restricting the scope of rewriting to the function signature. We can type check the new package using the imports of the old package. We can find the next reference in the new package by counting. Fixes golang#63534 Change-Id: Iba5fa6b0da503b7723bea1b43fd2c4151576a672 Reviewed-on: https://go-review.googlesource.com/c/tools/+/532495 Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Robert Findley <[email protected]>
Propagate goversions on functions and initializers. Updates golang#63374 Change-Id: I67b25b65fd83888fa27818aee3570b8bf4c09508 Reviewed-on: https://go-review.googlesource.com/c/tools/+/533239 TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Tim King <[email protected]> Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
The heuristic used to identify remaining uninlined calls by counting fails when the inlining operation changes the order of "original" calls, as may be the case with overlapping calls, following CL 535456. I spent around an hour trying to fix this properly (by tracking calls that were "just" inlined), but it's too hard because we must also track calls that had previously been inlined. It needs more theory and thought. For now, remove support for overlapping calls, as in all other cases the top-to-bottom counting heuristic should still work. For golang#63534 Change-Id: I59fe4aa5d20e56b0eb9dcbc01d29dd2ece082c17 Reviewed-on: https://go-review.googlesource.com/c/tools/+/535795 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Auto-Submit: Robert Findley <[email protected]>
Joint with Alan Donovan (CL 535455) :) For golang#63534 Change-Id: Ia4fdc617edf6699da285f56670a51efac1817834 Reviewed-on: https://go-review.googlesource.com/c/tools/+/534918 Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
This showed up when I used the inliner for removing parameters: we should be more precise about detecting the last use of a local var. A TODO is left to make this perfect, which will be done in a subsequent CL. For golang#63534 Change-Id: Id5c753f3e7ae51e07db1d29a59e82e51c6d5952c Reviewed-on: https://go-review.googlesource.com/c/tools/+/535335 Reviewed-by: Alan Donovan <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
This change fixes a bug that caused func() error { return nil }()
to be reduced to nil in some cases, because the expr-context
reduction strategy was abusing safeReturn, which is only
appropriate for tailcalls. (That function has been renamed
for clarity.)
The safeReturn condition in the expr-context strategy has
been pushed down to the leaves, allowing it to be relaxed
in some cases (e.g. simple tail calls) by materializing
conversions.
Also, tests.
There is more work to do to cleanly factor the reification
of implicit return conversions across strategies, and to
do it only when actually necessary.
Change-Id: I775554a0a3d1348f8dbd9930904edd819f7c3839
Reviewed-on: https://go-review.googlesource.com/c/tools/+/535796
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
… types Fixes golang#51261 Change-Id: Ic71f614acd3a78a9db73483ccefe613cb7f66e5d
Change-Id: Ib629c051a19705d2a1d32cbbcdef36813f047aa1
andig
force-pushed
the
vet/basic-types
branch
from
5846d37 to
a63c496
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #51261