Implement Type Simplification v2 #3318
Conversation
ghost
requested review from
go/types/simplify.go
Outdated
| return t, false | ||
| } | ||
|
|
||
| return newType(CompoundDesc{t.Kind(), newElemTypes}, tc.nextTypeId()), true |
There was a problem hiding this comment.
I'm not sure why we don't want to intern this?
go/types/simplify.go
Outdated
| for i, f := range desc.fields { | ||
| newType, c := removeAndCollectStructFields(tc, f.Type, seen, pendingStructs) | ||
| newFields[i] = StructField{Name: f.Name, Type: newType, Optional: f.Optional} | ||
| changed = changed || c |
There was a problem hiding this comment.
It doesn't look like we are using this changed. We should probably use it below to determine if we want to use the old desc.fields instead of newFields
There was a problem hiding this comment.
I fixed it but I'm not sure it is worth it here... Maybe it would be better to remove the change out param from removeAndCollectStructFields.
|
I'm OK with this approach. Defer to @arv for review. |
|
I see you removed both the skips from #3313 @rafael-atticlabs I added a commit to this PR... (by pushing to rafael-atticlabs/typeSimplification2) |
|
I'm landing this. There is some followup work that needs to be done but I'll do that in different PRs |
Fixes #3184