We no longer need to worry about the scope of range variables,
we can iterate over integers directly, and we can use cmp.Or too.
I haven't paid close attention to using these everywhere.
This is mainly testing out the new features where I saw some benefit.
Up until now, the new SSA reflection detection relied on call sites
to propagate which objects (named types, struct fields) used reflection.
For example, given the code:
json.Marshal(new(T))
we would first record that json.Marshal calls reflect.TypeOf,
and then that the user's code called json.Marshal with the type *T.
However, this would not catch a slight variation on the above:
var t T
reflect.TypeOf(t)
t.foo = struct{bar int}{}
Here, type T's fields such as "foo" and "bar" are not obfuscated,
since our logic sees the call site and marks the type T recursively.
However, the unnamed `struct{bar int}` type was still obfuscated,
causing errors such as:
cannot use struct{uKGvcJvD24 int}{} (value of type struct{uKGvcJvD24 int}) as struct{bar int} value in assignment
The solution is to teach the analysis about *ssa.Store instructions
in a similar way to how it already knows about *ssa.Call instructions.
If we see a store where the destination type is marked for reflection,
then we mark the source type as well, fixing the bug above.
This fixes obfuscating github.com/gogo/protobuf/proto.
A number of other Go modules fail with similar errors,
and they look like very similar bugs,
but this particular fix doesn't apply to them.
Future incremental fixes will try to deal with those extra cases.
Fixes#685.
A recent PR added a bigger regression test for go-spew,
and fixed an issue where we would obfuscate local named types
even if they were embedded into local structs used for reflection.
This would effectively mean we were obfuscating one field name,
the one derived from the embedding, which we didn't want to.
The fix did this by searching for embedded objects with extra code.
However, as far as I can tell, that isn't necessary;
we can do the right thing by recording all local type names
just like we already do for all field names.
This results in less complicated code, and avoids needing special logic
to handle embedding struct types, so I reckon it's a win.
Add even more tests to convince myself that we're still obfuscating
local types and field names which aren't used for reflection.
computeLinkerVariableStrinsg had an unusedargument.
Only skip obfuscating the name "FS" in the "embed" package.
The reflect methods no longer use the transformer receiver type,
so that TODO now feels unnecessary. Many methods need to be aware
of what the current types.Package is, and that seems reasonable.
We no longer use writeFileExclusive for our own cache on disk,
so the TODO about using locking or atomic writes is no longer relevant.
loadPkgCache also uses different pkgCache variables
depending on whether it finds a direct cache hit or not.
Now we only initialize an entirely new pkgCache
with the first two ReflectAPIs entries when we get a direct cache miss,
since a direct cache hit will already load those from the cache.
That is, stop reusing "transformer" as the receiver on methods,
and stop writing the results to the global curPkgCache struct.
Soon we will need to support computing pkgCache for any dependency,
not just the current package, to make the caching properly robust.
This allows us to fill reflectInspector with different values.
The explicit isolation also helps prevent bugs.
For instance, we were calling recursivelyRecordAsNotObfuscated from
transformCompile, which happens after we have loaded or saved pkgCache.
Meaning, the current package sees a larger pkgCache than its dependents.
In this particular case it wasn't causing any bugs,
since the two reflect types in question only had unexported fields,
but it's still good to treat pkgCache as read-only in transformCompile.
To properly make our cache robust, we'll need to be able to compute
cache entries for dependencies as needed if they are missing.
So we'll need to create more of these struct values in the code.
Rename cachedOutput to curPkgCache, to clarify that it relates
to the current package.
While here, remove the "known" prefix on all pkgCache fields.
All of the names still make perfect sense without it.
Per the TODOs that I left myself in the last commit.
As expected, this change allows tidying up the code a bit,
makes our use of caching a bit more consistent,
and also allows us to load the current package from the cache.
Daniel Martí
Paul Scheduikat
xuannv