If an interface contains a blank method, that's already an error. No
need for useless follow-up error messages about not implementing them.
Fixes#42964.
Change-Id: I5bf53a8f27d75d4c86c61588c5e2e3e95563d320
Reviewed-on: https://go-review.googlesource.com/c/go/+/275294
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
In the past, we had a lot of trouble with misusing *types.Sym
throughout the frontend, so I tried to push us towards always passing
around ONAMEs instead. But for constructing and writing out the symbol
indexes for the indexed export data, keying by *types.Sym is exactly
what we want.
Passes buildall w/ toolstash -cmp.
Updates #42982.
Change-Id: Idd8f1fb057d75a52a34ebc7788d9332fb49caf8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/275755
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
All of the uses were already using *ir.Func index operands, so only
needs the map type itself updated.
Passes buildall w/ toolstash -cmp.
Updates #42982.
Change-Id: I568d8601f3eb077e07e887f2071aa1a2667d803c
Reviewed-on: https://go-review.googlesource.com/c/go/+/275754
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Similar to the previous CL, the only two users of NodeQueue only
needed it for tracking objects, not arbitrary AST nodes. So change
it's signature to use *Name instead of Node.
This does require a tweak to the nowritebarrierrec checker, because
previously it was pushing the ODCLFUNC *Func pointers into the queue,
whereas now we push the ONAME/PFUNC *Name pointers instead. However,
it's trivial and safe to flip between them.
Also, this changes a handful of export-related code from Node to
*Name, to avoid introducing type assertions within iexport.go.
Passes buildall w/ toolstash -cmp.
Updates #42982.
Change-Id: I867f9752121509fc3da753978c6a41d5015bc0ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/275753
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
The only user of NodeSet (computing initialization dependencies) only
needs to store *Names in this structure. So change its definition to
match that need, and update the code in initorder.go accordingly.
Passes buildall w/ toolstash -cmp.
Updates #42982.
Change-Id: I181a8aaf9bc71e88f4ac009c4f381a718080e48f
Reviewed-on: https://go-review.googlesource.com/c/go/+/275752
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Currently, because we use the same *Name to represent both declaration
and uses of an object, it's ambiguous what "n1 == n2" means when
comparing two Node values. It can mean any of: Are these the same
syntactic element? Is n1 a use of declared variable n2? Are n1 and n2
both uses of the same declared variable?
We'd like to introduce a new IdentExpr node to replace use of Name
within the AST, but that means those three cases need to be handled
differently. The first case needs to stay "n1 == n2", but the other
cases need to become "n1.Name() == n2" and "n1.Name() == n2.Name()",
respectively. ("n1.Name() == n2.Name()" also currently works for the
second case, but eventually we'll want to get rid of the Name.Name
method.)
This CL introduces helper functions SameSource and Uses to handle
these cases. It also introduces DeclaredBy, which is another somewhat
common case that the next CL introduces uses of.
Passes buildall w/ toolstash -cmp.
Updates #42990.
Change-Id: Ia816c124446e9067645d5820a8163f295968794f
Reviewed-on: https://go-review.googlesource.com/c/go/+/275305
Reviewed-by: Russ Cox <rsc@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
The only reason for the DeepCopyNode interface was to
allow the type syntaxes to avoid being constrained by
Left, Right etc. methods. Now those are gone, so the
general traversal methods they implement (doChildren, editChildren)
do the right thing for DeepCopy.
Passes buildall w/ toolstash -cmp.
Change-Id: I54672c011114a95efabff32dbcf02e6071f91b9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/275379
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
deadcode is trying to walk the statements it can find,
but it can sweep in other nodes too. Stop doing that:
only walk known statements containing statements.
Otherwise, if we put panics in expression accessors that
shouldn't be used anymore, deadcode can trip them.
deadcode would be a good candidate to rewrite using
EditChildren, but that would certainly cause toolstash
changes, since deadcode is so ad-hoc about exactly
which parts of the function it looks at. For now just
remove the general traversal and leave as is.
Passes buildall w/ toolstash -cmp.
Change-Id: I06481eb87350905597600203c4fa724d55645b46
Reviewed-on: https://go-review.googlesource.com/c/go/+/275377
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Put each node in charge of its EditChildren implementation.
This removes the final generic use of Left, SetLeft, Right, SetRight,
and so on in package ir.
Passes buildall w/ toolstash -cmp.
Change-Id: I9821cc20f5b91cc9b44eb1f386cc82f20cd6770c
Reviewed-on: https://go-review.googlesource.com/c/go/+/275376
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Put each node in charge of its DoChildren implementation.
This removes a generic use of Left, Right, and so on
in func DoChildren, heading toward removing those even from
being used in package ir.
Passes buildall w/ toolstash -cmp.
Change-Id: Ibdf56f36801217cf24549e063da0078c1820a56b
Reviewed-on: https://go-review.googlesource.com/c/go/+/275375
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Put each node in charge of making copies of its own slices.
This removes a generic use of Body, SetBody, and so on
in func Copy, heading toward removing those even from
being used in package ir.
Passes buildall w/ toolstash -cmp.
Change-Id: I249b7fe54cf72e9d2f0467b10f3f257abf9b29b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/275374
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL rephrases the general inlining rewriter in terms of ir.EditChildren.
It is the final part of the code that was processing arbitrary nodes using
Left, SetLeft, and so on. After this CL, there should be none left except
for the implementations of DoChildren and EditChildren, which fall next.
Passes buildall w/ toolstash -cmp.
Change-Id: I9c36053360cd040710716f0b39397a80114be713
Reviewed-on: https://go-review.googlesource.com/c/go/+/275373
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL converts all the generic searching traversal to use ir.Find
instead of relying on direct access to Left, Right, and so on.
Passes buildall w/ toolstash -cmp.
Change-Id: I4d951aef630c00bf333f24be79565cc564694d04
Reviewed-on: https://go-review.googlesource.com/c/go/+/275372
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL introduces the general visitor functionality that will replace
the Left, SetLeft, Right, SetRight, etc methods in the Node interface.
For now, the CL defines the functionality in terms of those methods,
but eventually the Nodes themselves will implement DoChildren
and EditChildren and be relieved of implementing Left, SetLeft, and so on.
The CL also updates Inspect (which moved to visit.go) and DeepCopy
to use the new functionality.
The Find helper is not used in this CL but will be used in a future one.
Passes buildall w/ toolstash -cmp.
Change-Id: Id0eea654a884ab3ea25f48bd8bdd71712b5dcb44
Reviewed-on: https://go-review.googlesource.com/c/go/+/275311
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Now inlcopy and ir.DeepCopy are semantically the same,
so drop the inlcopy implementation.
Passes buildall w/ toolstash -cmp.
Change-Id: Id2abb39a412a8e57167a29be5ecf76e990dc9d3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/275310
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
DeepCopy is not called DeepSepCopy, so it should use Copy, not SepCopy.
Also, the old gc.treecopy, which became ir.DeepCopy, only copied
the Left, Right, and List fields - not Init, Rlist, Body - and I didn't
notice when I moved it over. A general utility function should of
course copy the whole node, so do that.
Finally, the semantics of Copy should not depend on whether a
particular child node is held directly in a field or in a slice,
so make Copy duplicate the slice backing arrays as well.
(Logically, those backing arrays are part of the node storage.)
Passes buildall w/ toolstash -cmp.
Change-Id: I18fbe3f2b40078f566ed6370684d5585052b36a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/275309
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The upcoming general iterators will process nodes in
source code order, meaning that the "then" block comes
before the "else" block. But for an if node, "then" is Body
while "else" is Rlist, and the inliner processes Rlist first.
The order of processing changes the order of inlining decisions,
which can affect which functions are inlined, but in general
won't affect much. (It's not like we know that we should prefer
to inline functions in else bodies over then bodies.)
Swapping these is not safe for toolstash -cmp.
Doing it in a separate CL lets the upcoming CLs all be toolstash-safe.
Change-Id: Id16172849239b0564930d2bbff1260ad6d03d5ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/275308
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL consolidates and cleans up fmt.go's logic for skipping past
Nodes introduced during typechecking. This allows eliminating SetOrig
on ConvExpr and Name. Also changes ConstExpr.SetOrig to a panic for
good measure.
The only remaining SetOrig uses now are for rewriting multi-value
"f(g())" calls and "return g()" statements, and type-checking
composite literals. It should be possible to eliminate both of those
as well.
Passes buildall w/ toolstash -cmp.
Change-Id: I478aea1a17dfb7a784293b930bf9081637eb2d7a
Reviewed-on: https://go-review.googlesource.com/c/go/+/275179
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
It's useful to have quick access to the types.Field that a given
selector or method value expression refer to. Previously we abused Opt
for this, but couldn't do that for OCALLPART because escape analysis
uses Opt.
Now that we have more flexibility, we can simply add additional
pointer fields for this. This also allows getting rid of an unneeded
ONAME node for OCALLPART.
Passes buildall w/ toolstash -cmp.
Change-Id: I980d7bdb19abfd0b6f58a232876861b88dee1e47
Reviewed-on: https://go-review.googlesource.com/c/go/+/275034
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
The next CL adds ConstExpr, which is a more memory efficient
representation for constant expressions than Name. However, currently
a bunch of Val helper methods are defined on Name. This CL changes
them into standalone functions that work with any Node.Val
implementation.
There's also an existing standalone function named Int64Val, which
takes a Type argument to specify what type of integer is expected. So
to avoid collisions, this CL renames it to IntVal.
Passes buildall w/ toolstash -cmp.
[git-generate]
cd src/cmd/compile/internal/ir
rf 'mv Int64Val IntVal'
sed -i -E -e 's/\(n \*Name\) (CanInt64|((I|Ui)nt64|Bool|String)Val)\(/\1(n Node/' name.go
cd ../gc
rf '
ex {
import "cmd/compile/internal/ir"
var n ir.Node
n.CanInt64() -> ir.CanInt64(n)
n.Int64Val() -> ir.Int64Val(n)
n.Uint64Val() -> ir.Uint64Val(n)
n.BoolVal() -> ir.BoolVal(n)
n.StringVal() -> ir.StringVal(n)
}
'
cd ../ir
rf '
mv CanInt64 Int64Val Uint64Val BoolVal StringVal val.go
rm Node.CanInt64 Node.Int64Val Node.Uint64Val Node.BoolVal Node.StringVal
'
Change-Id: I003140bda1690d770fd608bdd087e6d4ff00fb1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/275032
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Recreated manually to push below some CLs it depended on.
Change-Id: I1b3316fcdce39cbb33e5cbb471f5cd1cd2efc1f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/274599
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL improves handling of OCONVNOP nodes during ssa generation,
so it is not toolstash safe.
An OCONVNOP wrapper is necessary for the "for" condition of
certain compiled range loops, and the boolean evaluator was
not looking through them properly, generating unnecessary
temporaries. That change saved 8k of the (13 MB) go binary.
The other changes just streamline the handling of OCONVNOP
to be more like what OSTMTEXPR will be like. They have no
effect on output size but do tweak the ssa graph a little, which
causes different register decisions and therefore different output.
Change-Id: I9e1dcd413b60944e21554c3e3f2bdc9adcee7634
Reviewed-on: https://go-review.googlesource.com/c/go/+/274598
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The check for blank in okAs is redundant with what its callers already
done, so just inline the conversion in callers side instead.
Passes toolstash-check.
Change-Id: I606105e2d2cf8e80214722a13c3101c464d20d82
Reviewed-on: https://go-review.googlesource.com/c/go/+/274793
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Noticed the typ field was duplicated, since it is also in miniExpr inside Name.
Also clarified the comments for Func, now that it is actually the ODCLFUNC node.
Change-Id: Ia483a0ad34bb409cd92c43d4ae0a6852f9e4f644
Reviewed-on: https://go-review.googlesource.com/c/go/+/274619
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
For statements like goto that don't need an init, use an
explicit block statement instead of forcing them to have one.
There is also one call to addinit that is being replaced with
a block. That call is the source of much of my confusion
regarding init statements: walkstmt calls addinit on a statement,
whereas all the other uses of addinit are on expressions.
After this CL, they're all expressions.
Passes buildall w/ toolstash -cmp.
Change-Id: Ifdef9d318c236dc1a7567f9e9ef4a6bedd3fe81f
Reviewed-on: https://go-review.googlesource.com/c/go/+/274597
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Not toolstash -cmp safe, so split into its own CL.
Change-Id: I523227514a95e19c9afe17556d754a03164bce76
Reviewed-on: https://go-review.googlesource.com/c/go/+/274595
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
OEMPTY is an empty *statement*, but it confusingly
gets handled as an expression in a few places.
More confusingly, OEMPTY often has an init list,
making it not empty at all. Replace uses and analysis
of OEMPTY with OBLOCK instead.
Passes buildall w/ toolstash -cmp.
Change-Id: I8d4fcef151e4f441fa19b1b96da5272d778131d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/274594
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Now that filepath.WalkDir is available, it is more efficient
and should be used in place of filepath.Walk.
Update the tree to reflect best practices.
As usual, the code compiled with Go 1.4 during bootstrap is excluded.
(In this CL, that's only cmd/dist.)
For #42027.
Change-Id: Ib0f7b1e43e50b789052f9835a63ced701d8c411c
Reviewed-on: https://go-review.googlesource.com/c/go/+/267719
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
This allows directly creating an ONONAME, which is a primordial Name
before having its Op initialized. Then after an Op is assigned, we
never allow it to be reassigned.
Passes buildall w/ toolstash -cmp.
Change-Id: Ibc2f413dc68c0af6a96abfe653c25ce31b184287
Reviewed-on: https://go-review.googlesource.com/c/go/+/274620
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Allows getting rid of the SetPkg method and also addresses a
long-standing TODO in the exporter. Suggested by rsc@.
Passes buildall w/ toolstash -cmp.
Change-Id: Ib294f75f1350572efb2e0d993d49efef884de3d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/274440
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
It's noisy and not doing any harm, and we still have an entire release
cycle to revisit and address the issue properly.
Updates #42938
Change-Id: I1de5cfb495a8148c9c08b215deba38f2617fb467
Reviewed-on: https://go-review.googlesource.com/c/go/+/274732
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This CL adds the remaining constructors needed to abstract away
construction of Types, and updates the compiler to use them
throughout. There's now just a couple uses within test cases to
remove.
While at it, I also replace the Func.Outnamed field with a simple
helper function, which reduces the size of function types somewhat.
Passes toolstash/buildall.
Change-Id: If1aa1095c98ae34b00380d0b3531bd63c10ce885
Reviewed-on: https://go-review.googlesource.com/c/go/+/274713
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
There's no need for the bucket type to be precise. The compiler
doesn't actually generate code that references these fields; it just
needs it for size and GC bitmap calculations.
However, changing the type field does alter the runtime type
descriptor and relocations emitted by the compiler, so this change
isn't safe for toolstash.
Change-Id: Icf79d6c4326515889b13435a575d618e3bbfbcd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/274712
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
These replacement rules assume that TST and TEQ set V. But TST and
TEQ do not set V. This is a problem because instructions like LT are
actually checking for N!=V. But with TST and TEQ not setting V, LT
doesn't do anything meaningful. It's possible to construct trivial
miscompilations from this, such as:
package main
var x = [4]int32{-0x7fffffff, 0x7fffffff, 2, 4}
func main() {
if x[0] > x[1] {
panic("fail 1")
}
if x[2]&x[3] < 0 {
panic("fail 2") // Fails here
}
}
That first comparison sets V, via the CMP that subtracts the values
causing the overflow. Then the second comparison operation thinks that
it uses the result of TST, when it actually uses the V from CMP.
Before this fix:
TST R0, R1
BLT loc_6C164
After this fix:
TST R0, R1
BMI loc_6C164
The BMI instruction checks the N flag, which TST sets. This commit
fixes the issue by using [LG][TE]noov instead of vanilla [LG][TE], and
also adds a test case for the direct issue.
Fixes#42876.
Change-Id: I13c62c88d18574247ad002b671b38d2d0b0fc6fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/274026
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Just clearing away some scaffolding artifacts from previous
refactorings.
[git-generate]
cd src/cmd/compile/internal/gc
rf '
ex {
import "cmd/compile/internal/ir"
import "cmd/compile/internal/types"
var n *ir.Name; n.Name() -> n
var f *ir.Func; f.Func() -> f
var o types.Object
ir.AsNode(o).Sym() -> o.Sym()
ir.AsNode(o).Type() -> o.Type()
ir.AsNode(o).(*ir.Name) -> o.(*ir.Name)
ir.AsNode(o).(*ir.Func) -> o.(*ir.Func)
var x ir.Node
ir.AsNode(o) != x -> o != x
}
'
Change-Id: I946ec344bd7ee274900a392da53b95308ceaade4
Reviewed-on: https://go-review.googlesource.com/c/go/+/274592
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
There's not really any use to tracking function-scoped constants and
types on Curfn.Dcl, and there's sloppy code that assumes all of the
declarations are variables (e.g., cmpstackvarlt).
Change-Id: I5d10dc681dac2c161c7b73ba808403052ca0608e
Reviewed-on: https://go-review.googlesource.com/c/go/+/274436
Reviewed-by: Russ Cox <rsc@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
This CL introduces types.NewBasic, for creating the predeclared
universal types, and reorganizes how the universe is initialized so
that all predeclared types are uniformly constructed.
There are now a bunch of Type fields that are no longer assigned
outside of the package, so this CL also introduces some new accessor
methods that a subsequent CL will mechanically introduce uses of.
Change-Id: Ie7996c3d5f1ca46cd5bfe45ecc91ebfa6a7b6c7d
Reviewed-on: https://go-review.googlesource.com/c/go/+/274435
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
The start of abstracting away Type fields. This adds a new constructor
for named types, styled after go/types.NewNamed. Along with helper
methods for SetNod and Pos, this allows hiding Nod.
Change-Id: Ica107034b6346c7b523bf6ae2a34009e350a9aa8
Reviewed-on: https://go-review.googlesource.com/c/go/+/274434
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Now that setUnderlying is decoupled from Nodes, it can be moved into
package types, where it really belongs.
[git-generate]
cd src/cmd/compile/internal/gc
rf '
mv setUnderlying SetUnderlying
mv SetUnderlying typex.go
mv typex.go cmd/compile/internal/types
'
cd ../types
rf '
mv typex.go type.go
'
Change-Id: I76e2d4d8a6df599f24a731c4d8e5774ec83a119c
Reviewed-on: https://go-review.googlesource.com/c/go/+/274433
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
The code for type-checking defined types was scattered between
typecheckdef, typecheckdeftype, and setUnderlying. There was redundant
work between them, and setUnderlying also needed to redo a lot of work
because of its brute-force solution of just copying all Type fields.
This CL reorders things so as many of the defined type's fields are
set in advance (in typecheckdeftype), and then setUnderlying only
copies over the details actually needed from the underlying type.
Incidentally, this evidently improves our error handling for an
existing test case, by allowing us to report an additional error.
Passes toolstash/buildall.
Change-Id: Id59a24341e7e960edd1f7366c3e2356da91b9fe7
Reviewed-on: https://go-review.googlesource.com/c/go/+/274432
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Allows emitting errors about ineffectual //go:linkname directives.
In particular, this exposed: a typo in os2_aix.go; redundant (but
harmless) directives for libc_pipe in both os3_solaris.go and
syscall2_solaris.go; and a bunch of useless //go:linkname directives
in macOS wrapper code.
However, because there's also ineffectual directives in the vendored
macOS code from x/sys, that can't be an error just yet. So instead we
print a warning (including a heads up that it will be promoted to an
error in Go 1.17) to prevent backsliding while we fix and re-vendor
that code.
Passes toolstash-check.
Change-Id: I59badeab5df0d8b3abfd14c6066e9bb00e840f73
Reviewed-on: https://go-review.googlesource.com/c/go/+/273986
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
RawCopy breaks the invariant that ir.Orig depends on for
allowing nodes to omit keeping their own orig fields.
Avoid surprises by unexporting it.
The only use in package gc was removed in the previous CL.
This one is a straight global search and replace RawCopy -> rawCopy.
Change-Id: Ia99c0f4665bf7ed4f878cc44456d5fbdf33bab8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/274293
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
The ONIL export bug happened because the logic about
maintaining an “implicit” orig pointer in the comments
around ir.Orig only applies to Copy and SepCopy, not to
direct use of RawCopy. I'd forgotten those could exist.
The sole direct use of RawCopy was for the OLITERAL/ONIL case.
The ONIL is now provably indistinguishable from Copy, since
NilExpr does not have an explicit Orig field, so for NilExpr
RawCopy and Copy are the same.
The OLITERAL is not, but we can reconstruct the effect
with Copy+SetOrig to be explicit that we need the orig link.
The next CL will unexport RawCopy.
Also fix a typo in MapType doc comment.
Passes buildall w/ toolstash -cmp.
Change-Id: I876a85ff188e6d1cd4c0dfa385be32482e0de0d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/274292
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
When checking if a defined type is part of a type loop, we can
short-circuit if it was defined in another package. We can assume any
package we import already successfully compiled, so any types it
contains cannot be part of a type loop.
This also allows us to simplify the logic for recursing into the type
used in the type declaration, because any defined type from this
package will have a properly setup node.
Change-Id: Ic024814d95533afd9e59f2103c8ddb22bd87e900
Reviewed-on: https://go-review.googlesource.com/c/go/+/274294
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Matthew Dempsky
Cuong Manh Le
Russ Cox
Dan Scales
Jason A. Donenfeld