Adds a new versions package to provide x/tools a way to
deal with new GoVersion() and FileVersions API from go/types
and the new go/version standard library.
This provides a stable API until 1.26.
Updates golang/go#63374
Updates golang/go#62605
Change-Id: I4de54df00ea0f4363c0383cbdc917186277bfd0a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/533056
Run-TryBot: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The "golang.org/x/sys/execabs" package was introduced to address a
security issue on Windows, and changing the default behavior of os/exec
was considered a breaking change. go1.19 applied the behavior that was
previously implemented in the execabs package through CL 403274.
This reverts fe37c9e135 (CL 284773).
Updates #43724.
Change-Id: I53987d0d0009b8dd11e05fc3d17cbffb9625a9c1
GitHub-Last-Rev: 18e93f39c4
GitHub-Pull-Request: golang/tools#455
Reviewed-on: https://go-review.googlesource.com/c/tools/+/539017
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Commit-Queue: Bryan Mills <bcmills@google.com>
(following go list)
Change-Id: Ie87af973b853584082c298c339e995a8030f2d9b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/540475
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The -whylive flag explains why a function is live (not dead code),
by displaying an arbitrary shortest path to the function from
one of the main entrypoints. Call paths from main packages (not tests),
from main functions (not init functions), and involving only
static calls are preferred.
The output logic is unified so that the usual mode and -whylive
both compute a list of JSON objects, which are then formatted
by -json or -format=template.
Also, a test.
Also, fix the test framework to correctly handle a sequence of commands(!).
Fixesgolang/go#61263
Change-Id: I6aafc851c9c57e27a0a8f0d723e20fb2f8b57ad7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/540219
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change adds support for JSON output and text/template
formatting of output records, in the manner of 'go list (-f|-json)'.
Plus test.
Also, the -generated flag is now enabled by default,
and it affects all output modes. Plus test.
Updates golang/go#63501
Change-Id: I1374abad78d800f92739de5c75b28e6e5189caa1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/539661
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Port the fillstruct and self_assignment suggestedfix marker tests.
The fillstruct marker test in particular had incredibly verbose golden
content, which made the tests very difficult to read. To mitigate this,
introduce a new 'codeactionedit' marker which only stores edits in the
golden directory, rather than complete file content. Additionally,
narrow the unified diff to have no edges, for brevity. Since none of the
fillstruct tests require multi-line ranges, use a single location for
the range.
Furthermore, standardize on putting locations before action kind in code
action markers. This is more consistent with other markers.
For golang/go#54845
Change-Id: Id5d713b77fa751bfe8be473b19304376bc3bb139
Reviewed-on: https://go-review.googlesource.com/c/tools/+/539655
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Previously we used os.SameFile, but that is needlessly expensive when
we expect the original (user-provided) paths to be preserved during
the walk: the user-provided list of ignored directories is relative.
Because 'go list' does not traverse symlinks, we do not expect a
user's source roots to contain symlinks for Go package source either,
and in the rare even that the tree contains a directory that resides
within a non-ignored subdirectory symlink, the user can explicitly
include all of the relative paths by which the directory may be
encountered.
This reduces benchmark latency by ~7.5% compared to CL 508506, bringing
the overall latency to +~6% over the previous approach using
internal/fastwalk.
~/x/tools$ benchstat cl508506.txt cl508507.txt
goos: linux
goarch: amd64
pkg: golang.org/x/tools/internal/imports
cpu: AMD EPYC 7B12
│ cl508506.txt │ cl508507.txt │
│ sec/op │ sec/op vs base │
ScanModCache-24 264.4m ± 1% 244.6m ± 1% -7.49% (p=0.000 n=10)
~/x/tools$ benchstat cl508505.txt cl508507.txt
goos: linux
goarch: amd64
pkg: golang.org/x/tools/internal/imports
cpu: AMD EPYC 7B12
│ cl508505.txt │ cl508507.txt │
│ sec/op │ sec/op vs base │
ScanModCache-24 231.0m ± 1% 244.6m ± 1% +5.90% (p=0.000 n=10)
For golang/go#58035.
Change-Id: I937faf7793b8fad10a88b2fdc21fa4e4001c7246
Reviewed-on: https://go-review.googlesource.com/c/tools/+/508507
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
golang.org/x/tools/internal/fastwalk adds a lot of complexity to avoid
unnecessary calls to os.Stat. It also adds very limited concurrency
(a maximum of 4 goroutines).
filepath.WalkDir, added in Go 1.16, also avoids unnecessary calls to
os.Stat, and is part of the standard library (so it adds much less
complexity to x/tools). Switching to that substantially simplifies the
implementation of internal/gopathwalk, at the cost of losing the small
amount of concurrency added by fastwalk.
The internal/gopathwalk package does not appear to have any benchmarks
of its own, but the benchmark impact shows a ~14.5% increase in the
imports.ScanModCache benchmark on Linux with a warm filesystem cache,
but that can be reduced to ~5.5% by also removing an apparently
unnecessary call to os.SameFile (in followup CL 508507).
To me, the decrease in code complexity seems worth the slight increase
in latency.
~/x/tools$ benchstat cl508505.txt cl508506.txt
goos: linux
goarch: amd64
pkg: golang.org/x/tools/internal/imports
cpu: AMD EPYC 7B12
│ cl508505.txt │ cl508506.txt │
│ sec/op │ sec/op vs base │
ScanModCache-24 231.0m ± 1% 264.4m ± 1% +14.47% (p=0.000 n=10)
Fixesgolang/go#58035.
Change-Id: Iffcdab4e9aea11c0f0d7a87904935635e20b2fe7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/508506
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
While investigating a bug that turned out to lie elsewhere
(#63700), we were troubled by the inconsistencies between
the pair of errors returned by functions in this package.
This change makes things more consistent.
Updates golang/go#63700
Change-Id: I926c47572b7f666327bd1dba71ace68a5591bf2f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/537875
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
When analyzing callee objects, keep track of their package names so that
we don't have to assign explicit names to new imports when there is no
conflict with their implicit name.
Fixesgolang/go#63613
Change-Id: I5b8dc9c514e434fb4262cab321933a28729bbd76
Reviewed-on: https://go-review.googlesource.com/c/tools/+/536755
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Adds support for ssa to range over int types.
Fixesgolang/go#63373
Change-Id: I451b175d2c62958e022f1b6489971f1af32b19e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/535035
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
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 <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
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/go#63534
Change-Id: Id5c753f3e7ae51e07db1d29a59e82e51c6d5952c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/535335
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Joint with Alan Donovan (CL 535455) :)
For golang/go#63534
Change-Id: Ia4fdc617edf6699da285f56670a51efac1817834
Reviewed-on: https://go-review.googlesource.com/c/tools/+/534918
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
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.
Fixesgolang/go#63534
Change-Id: Iba5fa6b0da503b7723bea1b43fd2c4151576a672
Reviewed-on: https://go-review.googlesource.com/c/tools/+/532495
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Findley <rfindley@google.com>
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 <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Plus, a test.
Change-Id: I9d3f4729c1b8da51d771442d9c3f5909f608591e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/534895
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
It was expensive, and unnecessary if we can rely on
go/types and export data preserving source order (we can).
Fixesgolang/go#61443
Change-Id: I28d93c35f89eb751991c9d25aeb1c1904ba7b546
Reviewed-on: https://go-review.googlesource.com/c/tools/+/534139
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change makes empty composite literals of aggregate
(struct/array) types duplicable. Nonempty literals remain
nonduplicable on grounds of verbosity; and map and slice
literals are nonduplicable because they allocate a new
variable.
Change-Id: I9e2d778e004fb4743fd242c4e81d00e55830a6bd
Reviewed-on: https://go-review.googlesource.com/c/tools/+/534397
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The treatment of MethodVal and FieldVal selections should
mostly be consistent, as both apply the same implicit
field selection operations to the explicit receiver.
(Only the final implicit &v or *ptr operations differ.)
However, Selection.Indirect has a bug that masked this
similarity.
This change handles SelectorExpr more carefully throughout.
The indirectSelection method helper provides a non-buggy
implementation of Selection.Indirect.
The net result is that, in the callee effects analysis,
selection operations that don't indirect a pointer are pure,
and those that do indirect a pointer act like a read,
allowing them to commute with other reads.
Change-Id: I1500785a72d0b184e39ae0a51448a165789c3ca3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/533156
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
A call f() where f() has tuple type may appear as the
sole operand of a return statement. I forgot this case
in the "reduce spread-context call" strategy.
Plus a test.
Fixesgolang/go#63398
Change-Id: Ie851c977c3a2d237feabc95dbed4c50e6a1c96ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/533176
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Previously, the tail-call strategies required that the callee's
implicit return conversions must be trivial. That meant
returning a nil error (for example) defeated these strategies,
even though it is common in tail-call situations for the caller
function to have identical result types. For example:
func callee() error { return nil } // nontrivial conversion
func caller() error { return callee() } // identical result types
This change permits the tail-call strategies when the
callee and caller's results tuples are identical.
Fixesgolang/go#63336
Change-Id: I57d62213023861a2cfebed25b01ec28921efe441
Reviewed-on: https://go-review.googlesource.com/c/tools/+/533075
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change adds parens around the type in T(x) conversions
where T is a receive-only channel type, as previously it
would be misformatted as a receive of a receive.
Updates golang/go#63362
Change-Id: I935b5598d4bc3ea57dd52964e8b02005f5e6ef72
Reviewed-on: https://go-review.googlesource.com/c/tools/+/532576
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Following the edge case discovered in golang/go#60201, take a more
scientific approach to improving symbol match scoring:
- Add a conformance test that compares Matcher with SymbolMatcher,
querying all identifiers in x/tools. The two are not expected to agree
in all cases, but this test helped find interesting ranking edge
cases, which are added to the ranking test.
- Don't count a capital letter in the middle of a sequence of capital
letters (e.g. the M in YAML) as a word start. This was the
inconsistency that led to golang/go#60201.
- Compute the sequence bonus before role score; role score should take
precedent.
- Simplify the sequence scoring logic: a sequential character gets the
same score as a word start, unless it is the final character in the
pattern in which case we also adjust for whether it completes a word
or segment. This feels like a reasonable heuristic.
- Fix a bug in final-rune adjustment where we were checking the next
input rune for a segment start, not a separator.
Notably, the scoring improvements above were all derived from first
principles, and happened to also improve the conformance rate in the new
test.
Additionally, make the following cleanup:
- s/character/rune throughout, since that's what we mean
- add debugging support for more easily understanding the match
algorithm
- add additional commentary
- add benchmarks
Fixesgolang/go#60201
Change-Id: I838898c49cbb69af083a8cc837612da047778c40
Reviewed-on: https://go-review.googlesource.com/c/tools/+/531697
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
In calls from one method to another with a pointer receiver,
the inliner was inserting an unnecessary binding decl for
the receiver var, because it relied on Selection.Indirect,
but no actual indirection through the pointer is going on,
so the receiver is pure.
Instead, clear the purity flag only if there's a load
through an embedded field.
Also, fix a latent bug in the BlockStmt brace eliding
logic: we forgot to include function params/results
in scope when the block is the body of a function.
Plus tests for both.
Change-Id: I7e149f55fb344e5f733cdd6811d060ef0dc42883
Reviewed-on: https://go-review.googlesource.com/c/tools/+/532177
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Two recent CLs had a semantic (but non-syntactic) conflict,
so both passed CI independently but together tests fail.
The fix is to remove the redundant braces from test expectations
that the inliner no longer produces.
Change-Id: I4e6c755e992c6755be36f2d767e77ed70c358dd7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/532176
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When replacing a CallExpr beneath an ExprStmt,
a reduction strategy may return a BlockStmt containing
zero or more statements.
This change eliminates the braces for the block when
it is safe to do so, in other words when these
three conditions are met:
(a) the parent of the ExprStmt is an unrestricted
statement context e.g. a block or the body
of a case of a switch or select, but not, say
"if f(); cond {".
(b) there are no forward gotos in the caller that
may jump across a declaration. (Currently
we check for any control labels at all in the
caller.)
(c) there are no conflicts between names declared
in the callee block and in the caller block.
Plus tests.
Also, a fix and test for a latent bug allowing
reduction in a restricted "if stmt; expr" context.
Fixesgolang/go#63259
Change-Id: I558c75d8306dfd0679768cb4b3dbf05f14b23c39
Reviewed-on: https://go-review.googlesource.com/c/tools/+/532099
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This change adds an analysis of "fallible constant"
(falcon) expressions, and uses it to reject substitution
of parameters by constant arguments in the (rare) cases
where it is not safe.
When substituting a parameter by its argument expression,
it is (perhaps surprisingly) not always safe to replace
a variable by a constant expression, even of exactly
the same type, as it may turn a dynamic error into
a compile-time one, as in this example:
func f(s string) {
if len(s) > 0 { println(s[0]) }
}
f("") // inline
The call is replaced by:
if len("") > 0 {
println(""[0]) // error: constant index out of range
}
In general, operations on constants (in particular
-x, x+y, x[i], x[i:j], and f(x)) are subject to
additional static checks that don't make sense for
non-constant operands.
This analysis scans the callee function body for all
expressions that are not constant but would become
constant if the parameter vars were redeclared as
constants, and emits a constraint (a Go expression)
for each one that has the property that it will
not type-check (using types.CheckExpr) if the
particular argument values are unsuitable.
The substitution logic checks the constraints
and falls back to a binding decl if they are
not all satisfied. (More optimal solutions could
be found, but this situation is very uncommon.)
(It would be easy to detect these problems if we
simply type-checked the transformed source, but,
by design, we simply don't have the necessary
type information for indirect dependencies when
running in an environment like unitchecker.)
Fixesgolang/go#62664
Change-Id: I27d40adb681c469e9c711bf1ee6f7319b5725a2a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/531695
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The logic to choose a fresh names checked for conflicts
in the caller and callee but not against the newly added
imports. This change adds a fix and a test.
Also, don't use "init" as a PkgName: it's reserved.
Fixesgolang/go#63298
Change-Id: I7670fb841d6a4f521d567d39fef86c88f06e5c98
Reviewed-on: https://go-review.googlesource.com/c/tools/+/531698
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change causes parens to be inserted only if needed
around the new node in the replacement. Parens are needed
if the child is a unary or binary operation and either
(a) the parent is a unary or binary of higher precedence or
(b) the child is the operand of a postfix operator.
Also, tests.
Updates golang/go#63259
Change-Id: I12ee95ad79b4921755d9bc87952f6404cb166e2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/532098
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
There's a kink in the scope tree at Func{Decl,Lit}
that we forgot to straighten out in all cases.
Fixed, with test.
Change-Id: Iaed9636b4cc17537af6b607b153f8b4183ac7cb7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/531699
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
There is a certain amount of arbitrariness, but these constants
are common, fundamental, and short.
Change-Id: I198e8dbb511d25d969e35a4308108dcf89f79bb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/531696
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
We are down to one remaining call in all of x/tools
that doesn't inline correctly (due to golang/go#62664).
Change-Id: Ic39e60697323ede4565ce190bee69f670e627611
Reviewed-on: https://go-review.googlesource.com/c/tools/+/531456
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
In cross-package inlining, when replacing free names with
qualified identifiers, we failed to check for intervening
names defined in the callee that might cast a shadow over the
reference. This change re-uses the logic for shadow
detection when replacing parameters with arguments.
Plus tests.
Fixesgolang/go#62667
Change-Id: Ib6a620ed2cde313bf51d5fb8a0cd9363f9eadf6e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/531455
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This change fixes a bug described in a TODO comment in the
logic for discarding the results of arguments evaluated for
effect in a call that has both a receiver and a spread call.
Also, a test.
Change-Id: I9fae202a1eef8f7c130048f9cf655af91b1a67fe
Reviewed-on: https://go-review.googlesource.com/c/tools/+/531416
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change slightly refines the crude check for whether an
argument might contain the last reference to a caller local
var (and thus cannot safely be eliminated). It now checks
that the variable is defined within the body of the caller
(i.e. is not a parameter or result, or a global).
Also, tests.
Also, properly group the remaining test cases from the
CL that split the test table. (Some had been left in a
function called "JJJ", as a sign of unfinished business.)
Change-Id: Id79bea69176f6da8991f194fa1c3d1813e92fcfc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/531415
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
It was prone to merge conflicts since every CL appends
items to the table and they all have a similar prefix,
which confuses diff.
The new smaller tables are scoped by TestXYZ functions
that impose a modicum of organizational sanity.
No logical changes to test.
Change-Id: I5f09477b915d3c62443c770fa87881ac0ad26c7a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/530600
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Previously, we rejected arguments as substitution candidates
if their type (or its types.Default) did not exactly match
the parameter type. Now, we allow those substitutions to
proceed but we wrap the argument in an explicit conversion
so that the meaning of the program doesn't change.
We had initially been reluctant to do this out of concern
that it could cause a single conversion during argument
passing to be moved into a loop, where its dynamic cost
might be multiplied; for example, a string->any conversion
allocates memory. But we decided that this modest dynamic cost
is acceptable so long as the meaning does not change.
Also, this change includes a fix for golang/go#63193, in which the
type inferred for the argument expression in func(int16){}(1)
is not "untyped int" but "int16". In other words, the type
checker has incorporated knowledge of the parameter type.
It is unsafe to assume that the expression 1 will have the
same type in a different context, so we recompute the type
of each argument expression in a neutral context (using
CheckExpr).
Fixesgolang/go#63193
Change-Id: I0fd072cc7611d113af77193f6d3362268d9af158
Reviewed-on: https://go-review.googlesource.com/c/tools/+/530975
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
This changes adds an analysis of the order of evaluation
of parameter variables and other read or write effects
in the callee, and uses it to optimize inlining by
allowing parameter substitution of argument expressions
that are impure (modelled as reads, R) or have side effects
modelled as writes, W), so long as this does not cause
reordering of R-W, W-W, or W-R pairs of operations.
R-R pairs of operations may be reordered freely.
Pure expressions, which are neither R nor W, may be
ordered arbitrarily.
The caller-side hazard algorithm is due to Rob Findley.
Also:
- tests of parameter substitution in the presence of
reads and effects;
- unit test of callee effects algorithm.
- update TODO comments re: ParenExpr removal.
Change-Id: If0342f4b1edc22c8b3445e39b54403d5b58eb33b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/530655
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Previously the condition for eliminating named result vars
was that they do not escape, but that's not strong enough:
it should be that they are never referenced. If they are
referenced, they can be accommodated in a binding decl,
as done in this change.
Also, tests for the interaction of named results with
each strategy.
Change-Id: I2a4bdeec17c98efd488f9939aebe6ab1d72a0814
Reviewed-on: https://go-review.googlesource.com/c/tools/+/530438
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This CL is a minor cleanup. There's no need for the
Callee.BodyIsReturnExpr field now that the caller has
syntax (but not types) for the callee and has
the pair of Callee.{Total,Trivial}Returns.
In general, only type-derived information needs to
be recorded in the Callee.
Change-Id: Ib9c7661fb113bc043154bee59bf7cae872ad6691
Reviewed-on: https://go-review.googlesource.com/c/tools/+/530815
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Plus a test.
Change-Id: I2d5542ed2c6e0cabae740279d09ceaf0d22b8710
Reviewed-on: https://go-review.googlesource.com/c/tools/+/529877
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Somehow I overlooked this case.
Change-Id: I95471bd2254ca557f069993fdb4f2ad2908ec78a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/529876
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Tim King
Sebastiaan van Stijn
Alan Donovan
Rob Findley
Bryan C. Mills