Various fixes to the archetype builder's same-type-to-concrete handling #5826
Conversation
|
@swift-ci please smoke test and merge |
|
@DougGregor BTW, the "Mark same-type constraints on nested types as redundant" commit seems to have a very positive effect on the length of mangled names. If I measured correctly, the length of the longest mangled symbols in the stdlib is now about 2.5x-3x shorter. |
|
@DougGregor It would be nice to add a fix for rdar://29288428 here as well, as it touches the same function in the ArchetypeBuilder. And please see my comments on the rdar://29295062 regarding a possible improvement. These comments are too long to put them here. |
|
Note that my first commit removed the notion of "outer" requirement source (we use "explicit" now, which can of course be marked redundant), so I suspect the mangling improvements you noticed are from a combination of that commit and the second commit. |
"Explicit" covers everything we need now. NFC
…undant. When a same-type constraint equates a potential archetype with a same-type constraint, it implies equality of the nested types with the appropriate witnesses. However, these were getting marked as "explicit" when they should be "redundant". Fixes rdar://problem/29295062.
…-concrete requirements. Fixes rdar://problem/29279577.
This was a blatant hole in our checking, admitting ill-formed generic signatures that would crash down the line. Fixes rdar://problem/29288428.
DougGregor
force-pushed
the
archetype-builder-same-types
branch
from
6df47c6 to
be791e5
Compare
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
|
@DougGregor Doug, I tried with this PR and my comments regarding rdar://29295062 are still valid. Yes, you removed "outer", but this is not the actual problem. The problem is that when we add new requirements, we should mark any (doesn't matter if they are explicit or not) no longer necessary requirements as redundant. And this does not happen currently. In particular, any requirements added by means of addGenericSignature are marked as "explicit" and stay in the signature forever, even if they are not needed anymore. |
|
@DougGregor I created a new radar rdar://29311216 for this issue. I also managed to reduce the test-case to just a couple of lines, which do not depend on the stdlib. |
|
@DougGregor I updated the rdar://29295062. Your fix seems to be too aggressive and may loose some of the requirements on other archetypes in certain cases. |
2 participants
Fixes rdar://problem/29295062, rdar://problem/29279577, and rdar://problem/29288428.
Also reduces the size of the standard library binary by 68k.