This commit includes two changes:
* It makes unexported interface methods package-private, so that it's
not possible to type-assert on an unexported method in a different
package.
* It makes the globals used to identify interface methods defined
globals, so that they can (eventually) be left in the program for an
eventual non-LTO build mode.
These two passes are related, but can definitely work independently.
Which is what this change does: it splits the two passes. This should
make it easier to change these two new passes in the future.
This change now also enables slightly better testing by testing these
two passes independently. In particular, the reflect lowering pass got
some actual tests: it was barely unit-tested before.
I have verified that this doesn't really change code size, at least not
on the microbit target. Two tests do change, but in a very minor way
(and in opposite direction).
Sometimes, LLVM may rename named structs when merging modules.
Therefore, we can't rely on typecodeID structs to retain their struct
names.
This commit changes the interface lowering pass to not rely on these
names. The interp package does however still rely on this name, but I
hope to fix that in the future.
Previously there was code to avoid impossible type asserts but it wasn't
great and in fact was too aggressive when combined with reflection.
This commit improves this by checking all types that exist in the
program that may appear in an interface (even struct fields and the
like) but without creating runtime.typecodeID objects with the type
assert. This has two advantages:
* As mentioned, it optimizes impossible type asserts away.
* It allows methods on types that were only asserted on (in
runtime.typeAssert) but never used in an interface to be optimized
away using GlobalDCE. This may have a cascading effect so that other
parts of the code can be further optimized.
This sometimes massively improves code size and mostly negates the code
size regression of the previous commit.
This distinction was useful before when reflect wasn't properly
supported. Back then it made sense to only include method sets that were
actually used in an interface. But now that it is possible to get to
other values (for example, by extracting fields from structs) and it is
possible to turn them back into interfaces, it is necessary to preserve
all method sets that can possibly be used in the program in a type
assert, interface assert or interface method call.
In the future, this logic will need to be revisited again when
reflect.New or reflect.Zero gets implemented.
Code size increases a bit in some cases, but usually in a very limited
way (except for one outlier in the drivers smoke tests). The next commit
will improve the situation significantly.
I ran into an issue where I did a method call on a nil interface and it
resulted in a HardFault. Luckily I quickly realized what was going on so
I could fix it, but I think undefined behavior is definitely the wrong
behavior in this case. This commit therefore changes such calls to cause
a nil panic instead of introducing undefined behavior.
This does have a code size impact. It's relatively minor, much lower
than I expected. When comparing the before and after of the drivers
smoke tests (probably the most representative sample available), I found
that most did not change at all and those that did change, normally not
more than 100 bytes (16 or 32 byte changes are typical).
Right now the pattern is the following:
switch typecode {
case 1:
call method 1
case 2:
call method 2
default:
nil panic
}
I also tried the following (in the hope that it would be easier to
optimize), but it didn't really result in a code size reduction:
switch typecode {
case 1:
call method 1
case 2:
call method 2
case 0:
nil panic
default:
unreachable
}
Some code got smaller, while other code (the majority) got bigger. Maybe
this can be improved once range[1] is finally allowed[2] on function
parameters, but it'll probably take a while before that is implemented.
[1]: https://llvm.org/docs/LangRef.html#range-metadata
[2]: https://github.com/rust-lang/rust/issues/50156
Ayke van Laethem
Jaden Weiss