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>
The problems found by everything_test were in some cases
spurious, caused by its failure to ignore line directives.
Change-Id: I1501d9858e0c877f0488580c28aa7d636a004bd1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/530439
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This CL is a refactoring of the inline function,
extracting steps such as gathering of arguments and of
parameters, substitution, creation of the binding decl,
and the callerLookup, into top-level functions.
Other than code motion and doc tweaks, the only
significant change is the movement of callerPath
to a field Caller.path, allowing Caller.lookup to
be expressed as a method. This means the Caller
object _is_ mutated by the operation, but only
its internal state.
Change-Id: I5bd2d463d01765e553186a796300b12c7fed0dd9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/530056
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This change factors the escape analysis out of
AnalyzeCallee so it can be used on the caller side too,
where it provides information to the pure() predicate
on which local variables enclosing the call have
the "single assignment" property, meaning they are
never updated once they are initialized.
Expressions that read only single-assignment variables
may be safely considered pure.
Also, the definition of pure has been refined to
mean an expression that does not depend on the value
of any mutable variable. This disentangles it from
the related concept of having no effect, which is about
updating (not reading) variables. We introduce a separate
predicate for that. A follow-up change will make richer
use of them for analysis of effects in the calleee.
Also:
- plumb a logger into AnalyzeCallee too.
The logger is now mandatory.
- binary operators are now pure.
- a few tests have regressed in tidiness due to
the stricter definition of pure, but this will
be fixed by a follow-up analysis of callee effects.
Change-Id: I5256bcff86829dd02067ce2ef81b8cb58faa1f50
Reviewed-on: https://go-review.googlesource.com/c/tools/+/530055
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
1. move the package doc into doc.go
2. s/elimination/substitution/g, since parameters can be
eliminated by a binding decl too.
Change-Id: Ie9c893b3f45a237a024470f46914941f6cfb28a5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/529616
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This change adds a test that applies the inliner to every
function in one or more packages, and tests that it does
something sensible: either report an error, or produce
a valid code transformation.
It discovered a number of bugs even when applied to just
a single package, and I plan to run it over all of x/tools,
then all of k8s, then over the corpus of the module proxy
using our pipeline for parallel processing.
Change-Id: I70660d36406bf1a03b4c97114a29ca7629e5d279
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528495
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>
Line directives should be ignored (at both caller
and callee) as they would otherwise cause the
inliner to make a mess of the caller file.
This change ignores them consistently in the tests,
and adds a heuristic assertion to reject inputs
that seem to be affected by line directives.
Plus: a test.
Change-Id: I1a9bffd935fd7b288c47647304a2a6529779434b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528298
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
std contains packages named internal/foo, which are a corner
case in the internal package check because (a) they have
no leading slash and (b) the parent directory "" is a prefix
of everything.
Plus, a test.
Change-Id: I89cd9c8aab38115c5e8f4862c56139599e43d50c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528360
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>
Although ioutil.ReadDir is deprecated, the suggested replacement has
a different signature, returning []fs.DirEntry rather than []fs.FileEntry.
Some of the places where this occurs are better left referring to ioutil,
as build.Config wants the old behavior. (go/buildutil/util.go and
godoc/vfs/os.go) When someday build gets updated, these can be easily
found, and changed.
Some of the place use Mode().IsRegular() (cmd/toolstash/mail.go and
internal/fastwalk/fastwal_portable.go) and the code needs a minor adjustment.
And, happily, in all the other places one can use os.ReadDir directly
as only Name() is called.
There are no remaining instances of the generated ioutilReadDir().
Change-Id: I165ca27eafe2fe37fdf14390543b21f7e198281e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528135
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Peter Weinberger <pjw@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Plus a test.
Change-Id: I30b96c231dcfce5cb20972eb6d1822f83cb45faf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528556
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 x.f(args) may implicitly select embedded fields x.A.B.C.f(args).
Similarly, T.f(x, args) may call C.f(x.A.B.C, args).
In both cases there may be implicit pointer traversals.
This change updates the receiver logic to handle these cases
correctly and make the selections explicit. This may fail
if any of A, B, C is unexported from a different package.
(The bugs were unearthed by the "everything test",
to follow in CL 528495.)
Also, tests.
Change-Id: I843b45af85a31ae5b3a8d0ab029d328f4323de05
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528555
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
I forgot to do this in the CL that implemented
binding decls.
Change-Id: Idf3fcb5aac8e253e63ae599bc38f1eef38db2beb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528675
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Plus a test.
Change-Id: Icdee9e7a4f095074f9c5b9e93d4b53950a824fd6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528358
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This greatly improves the diffs obtained from typical inliner
refactorings, which involve one change to the imports
and one change to a function call, with a large gap in
between.
How does the cost relate to this parameter?
Do I hear any advance on 100?
Change-Id: Ia006ebf3374f9894d4c561d61853e1adaa47dca6
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528359
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Another bug unearthed by the everything test.
Also, a regression test.
Change-Id: I4b45e83c1fee7fd461f366fc25ef69512300aa74
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528616
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
A binding declaration is a var decl that declares a
set of parameter variables and assigns them to their
argument values. The vars are divided into specs of the
same type in the same way that parameters are grouped
into fields. Types are explicit, so that the correct
assignment conversions occur.
A binding decl allows many reduction strategies to
work even when some parameters were not eliminated
by substitution, or when some result vars escape,
because it keeps the cardinality of variables unchanged.
For example, the call f(a, b, c)
where: func f(x, y T, z U) { stmts }
can be reduced to:
{
var (
x, y T = a, b
z U = c
)
stmts
}
It is not always possible to create a binding decl.
For example, if U has "x" among its free names, then
the declaration of x above shadows the correct x
needed by U.
Strategies have been updated to insert a binding decl
where appropriate, and tests updated.
Change-Id: I1c7837df6f2e26a4426758fa07b066d378428221
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528175
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
1. We forgot to descend into CompositeLit.Type.
2. We forgot to recursively visit subtrees.
These bugs were unearthed by an "everything" test,
which inlines every function call and type-checks
the result, to follow.
Change-Id: I17d79134cb8c4b088b017e6022508bc233ad2454
Reviewed-on: https://go-review.googlesource.com/c/tools/+/528396
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change analyzes variadic functions more systematically.
There are three call forms: ordinary, ellipsis, and spread.
In the first two cases, the variadic parameter is
simplified to an ordinary slice, enabling parameter
elimination. (The ellipsis case was already done.)
The spread case cannot be simplified.
Also:
- tests.
- fix a bug in cloneNode: avoid Set(nil)
Change-Id: I1d34022558d251bf62cbbb84aab1752c7f8f88b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/527677
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change causes f(slice...) calls to be reduced
to f(slice), and the corresponding parameter
func(...T) to be replaced by func([]T), enabling
parameter elimination and reduction.
This is one of several baby steps towards handling
the richness of variadic and spread calls.
Also:
- split the arrays of params and results in
AnalyzeCallee; no need for Kind field.
- gather parameters (incl. receiver) in the
same manner as we gather arguments,
and set them to nil (just like arguments)
when we eliminate them.
- Enable inliner logging in gopls.
Curious users can read the log to find out
why the inliner chose a given strategy.
Also, tests.
Change-Id: I2ba7dbf284c1de8c4fb0c16ef77b8df7f33398da
Reviewed-on: https://go-review.googlesource.com/c/tools/+/527676
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>
Alan Donovan
Rob Findley
Peter Weinberger