Blocks dominated by "if false" should be retained in the
initial SSA form so they remain visible to subsequent source
code analysis tools.
In any case, true compilers already need a stronger version of
this optimization so they can simplify CFGs such as this:
const x, y = ...
switch x {case y:...}
where a branch is constant but the comparison of constants
does not occur within an expression.
LGTM=gri
R=gri
CC=golang-codereviews, pcc
https://golang.org/cl/101250043
The SSA builder shouldn't be in the business of
interprocedural optimization, especially in the presence of
concurrency.
This causes the instruction count to increase by 0.03%.
LGTM=gri
R=gri, pcc
CC=golang-codereviews
https://golang.org/cl/105020045
It was making the unsound assumption that cgn==nil => v is one
of {Global,Function,Const,Capture} to avoid checking v's type,
which is what it now does. This caused more expensive
constraints to be generated, which is suboptimal though not
wrong exactly.
In one benchmark, this change reduces the number of complex
constraints by about 23% of loads and 53% of stores, and
increases the number of (simple) copy constraints by about 5%.
LGTM=crawshaw
R=crawshaw
CC=golang-codereviews
https://golang.org/cl/106940043
Files that import "C" are not valid Go source files and
require preprocessing. Until now, the loader has simply
hard-coded CGO_ENABLED=0 (in effect) which causes go/build to
use build tags select pure Go implementations where they exist
(e.g. in $GOROOT). Where they don't (e.g. arbitrary user
code) this leads to masses of spurious type errors.
(Reported by Guillaume Charmes, private correspondence.)
This change causes the loader to invoke the cgo preprocessor
on such files and to load the preprocessed files instead,
using the original names. This means that the syntax offset
position information is garbage, although thanks to //line
directives, the line numbers at least should be good.
See comment in cgo.go for details.
This CL changes the loader's default behaviour and may make it slower.
CGO_ENABLED=0 enables the old behaviour.
Tested via stdlib_test, which now loads all standard packages
using cgo, and also exercises CGO_ENABLED=0 for "net" and "os/user".
LGTM=gri
R=gri, rsc
CC=golang-codereviews, guillaume.charmes
https://golang.org/cl/86140043
Also:
- extend Parent() to all Values and add to interface:
(Builtin/Const/Global => nil; Function => Enclosing)
- hide Function.Enclosing since it's now redundant wrt Parent()
- make (*Function).String robust for synthetics without pkg object
LGTM=gri
R=gri
CC=golang-codereviews, khr
https://golang.org/cl/87580044
This optimization reduces solve time (typically >90% of the
total) by about 78% when analysing real programs. It also
makes the solver 100% deterministic since all iterations are
ordered.
Also:
- remove unnecessary nodeid parameter to solve() method.
- don't add a fieldInfo for singleton tuples (cosmetic fix).
- inline+simplify "worklist" type.
- replace "constraintset" type by a slice.
LGTM=crawshaw
R=crawshaw
CC=golang-codereviews
https://golang.org/cl/95240043
Until now, the same Function was used to represent a method
(T)func() and the "method expression" function func(T) formed
from it. So the SSA code for this:
var buf bytes.Buffer
f := Buffer.Bytes
f(buf)
buf.Bytes()
would involve an implicit cast (ChangeType) on line 2.
However, compilers based on go/ssa may want to use different
calling conventions for them, like gccgo does (see issue
7839). This change decouples them by using an anonymous
function called a "thunk", rather like this:
f := func(r *bytes.Buffer) []byte { return r.Bytes() }
Thunks are similar to method wrappers; both are created by
makeWrapper.
"Interface method wrappers" were a special case of thunks for
direct calls (no indirection/fields) of interface methods.
They are now subsumed by thunks and have been deleted. Now
that only the needed thunks are built, we don't need to
populate the concrete method sets of interface types at all,
so (*Program).Method and LookupMethod return nil for them.
This results in a slight reduction in function count (>1%) and
instruction count (<<1%).
Details:
go/ssa:
- API: ChangeType no longer supports func/method conversions.
- API: (*Program).FuncValue now returns nil for abstract
(interface) methods.
- API: (*Function).RelString simplified.
"$bound" is now a suffix not a prefix, and the receiver
type is rendered package-relative.
- API: Function.Object is now defined for all wrappers too.
- API: (*Program).Method and LookupMethod return nil for
abstract methods.
- emitConv no longer permits (non-identical)
Signature->Signature conversions. Added assertion.
- add and use isInterface helper
- sanity: we check packages after Build, not Create, otherwise
cross-package refs might fail.
go/pointer:
- update tests for new function strings.
- pointer_test: don't add non-pointerlike probes to analysis.
(The error was checked, but too late, causing a panic.)
- fixed a minor bug: if a test probe print(x) was the sole
reference to x, no nodes were generated for x.
- (reflect.Type).MethodByName: updated due to ssa API changes.
Also, fixed incorrect testdata/funcreflect.go expectation
for MethodByName on interfaces.
oracle:
- fix for new FuncValue semantics.
- a "pointsto" query on an I.f thunk now returns an error.
Fixesgolang/go#7839
LGTM=gri
R=gri
CC=golang-codereviews, pcc
https://golang.org/cl/93780044
- Replaced check.initDependencies with check.initOrder;
this is the only semantic change, it affects only the
value of Info.InitOrder.
- Added additional init order test cases and adjusted
existing tests.
- Moved orderedSetObjects from resolver.go to ordering.go.
Fixesgolang/go#7964.
LGTM=adonovan
R=adonovan
CC=golang-codereviews
https://golang.org/cl/91450043
Revision 0ea4058a1ca3 caused the node numbering to become
sparse, violating a precondition of
(*callgraphResult).toSerial. Now we renumber the callgraph
nodes always, not just the qpos != nil case.
Fixesgolang/go#8171
LGTM=crawshaw
R=crawshaw
CC=golang-codereviews
https://golang.org/cl/103240044
This will fix the images in Brad's GoCon presentation.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/100950043
Also fixes the following nits;
- literal IPv6 address handling
- URL host component handling in the case of a wildcard listen
- URL port component handling in the case of no port component in origin
Fixesgolang/go#8096.
LGTM=dan.kortschak, adg
R=adg, golang-codereviews, dan.kortschak
CC=golang-codereviews
https://golang.org/cl/102770046
If you have multiple runs in old.txt and new.txt
the default behavior is to match them up pairwise
and compare successive pairs (and if you have a
different number of runs in each file, benchcmp
refuses to do anything).
The new -best flag changes the behavior to instead
compare the fastest run of each benchmark from
the two files. This makes sense if you believe that
the fastest speed is the 'actual' speed and the slower
results are due to the computer spending time doing
non-benchmark work while the benchmark was
running.
LGTM=josharian
R=golang-codereviews, josharian
CC=golang-codereviews
https://golang.org/cl/102890047
Programs such as this cause the PtrTo solver to attempt to
enumerate an infinite set of types {T, *T, ..., *******T, etc}.
t := reflect.TypeOf(T{})
for {
t = reflect.PtrTo(t)
}
The fix is to bound the depth of reflectively created types at
about 4 map/chan/slice/pointer constructors.
+ test.
LGTM=gri
R=gri
CC=crawshaw, golang-codereviews
https://golang.org/cl/102030044
This consistently yields better performance with go/pointer.
Also: return int not word from ntz().
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/97570044
go/types doesn't correctly round the largest possible
float32 literal values and fails. Instead of relying
on Rat.Float64 and float32 conversion, we need a
Rat.Float32 implementation with correct rounding.
LGTM=adonovan
R=adonovan
CC=golang-codereviews
https://golang.org/cl/96540045
They were disabled by mistake during the move to go.tools.
LGTM=dan.kortschak
R=golang-codereviews, dan.kortschak
CC=golang-codereviews
https://golang.org/cl/98440048
Fixes 7866. Adds a test case.
When trying to add newlines before certain imports via text manipulation,
a regex is used to iterate over all imports. The regex failed to match
dot imports because \w doesn't match a literal dot. This changes the regex
to accept a dot as well.
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/99400043
(I forgot about this when we added support for negative elements generally.)
We use floating point for negative numbers. The order of the
output is reversed from the previous (little-endian) behaviour
since it makes for more readable floating point.
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/95570043
The issue occurs only when deleting an import that has a blank line immediately preceding,
and other imports before that.
Currently, DeleteImport assumes there's a blank line-sized hole left behind
where the import was, and always deletes it. That blank line-sized hole is there in all cases
except the above edge case.
This fix checks for that edge case, and does not remove the blank line-sized hole.
The CL also adds a previously failing test case that catches this scenario. After the change to
DeleteImport, the new test passes (along with all other tests).
Fixesgolang/go#7679.
Note that there is no attempt to ensure the result *ast.File and *token.FileSet are perfectly
matching to what you would get if you printed the AST and parsed it back. This is how the
rest of the package and the current tests work (i.e., they only check that printing the AST gives
the correct output).
Changing that is very hard, if not impossible, at least not
without resorting to manipulating AST via printing, text manipulation and parsing.
This is okay for most usages, but it does create potential problems. For example,
astutil.Imports() currently only works correctly on freshly parsed AST. If that AST
is manipulated via astutil funcs, then Imports() may not always generate correct
output. However, thas is a separate issue and should be treated as such.
LGTM=bradfitz
R=golang-codereviews, gobot, adonovan, bradfitz
CC=golang-codereviews
https://golang.org/cl/92250045
It was very ugly; a little tweaking helps godoc parse it better.
Also make unsafeptr.go not own the package doc (add a blank line)
and put one more sentence about that check into doc.go.
Fixesgolang/go#7925.
LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/98370044
This is both easier to read and 25% shorter (helpful when
using String() as a map key for interning sets).
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/96370045
Very long instructions caused the printf width spec to go
negative, which causes right-padding, often several lines'
worth.
Also: print the basic block comment once on the RHS. It's too
verbose to print it each time we mention the block.
LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/97490046
