Improve the mangling of USRs #8767
Conversation
|
@swift-ci Please test |
lib/AST/ASTMangler.cpp
Outdated
There was a problem hiding this comment.
Maybe we should add a getUnboundGenericType() to TypeDecl. You already had the same bit of code in an earlier PR today, and I've written it out in a few places also.
There was a problem hiding this comment.
Fine with me. Does it return UnboundGenericType, or just Type to handle the non-generic case?
There was a problem hiding this comment.
I think the latter. No need to do that before merging the PR of course.
lib/AST/ASTMangler.cpp
Outdated
There was a problem hiding this comment.
Oops, yes, that's the next patch I'm working on. Thanks.
lib/Demangling/Demangler.cpp
Outdated
There was a problem hiding this comment.
Maybe rename demangleNominalType()?
There was a problem hiding this comment.
I wish we had a better name for "nominal or typealias" than "generic type".
jrose-apple
force-pushed
the
mangle-typealiases-better
branch
2 times, most recently
from
2444052 to
a565c9b
Compare
|
Both full tests passed, and the only things since then have been renames and removing commented-out code. @swift-ci Please smoke test |
jrose-apple
force-pushed
the
mangle-typealiases-better
branch
from
a565c9b to
d3425ff
Compare
|
Added additional verification, fixed a bug that would have been caught by that verification, and updated ABI.rst. @swift-ci Please test |
|
Build failed |
|
Build failed |
jrose-apple
force-pushed
the
mangle-typealiases-better
branch
from
d3425ff to
c7f9cb7
Compare
jrose-apple
changed the title
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
@shahmishal, any idea why the failure isn't getting posted back on GitHub? |
|
It should have been 82e76edbb7170b0fce95eed922fb7e50bfea9bc0 not c7f9cb7548c51da7db2d21efe7d28478c8ee8e78. |
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
jrose-apple
force-pushed
the
mangle-typealiases-better
branch
from
c7f9cb7 to
d1efe11
Compare
This lets the mangling preserve the invariant that functions always structurally have function types. Error types don't show up in mangled names often anyway, but it can occur when you ask for the USR of a function with an invalid type.
...and /do/ produce them for 'self' parameters in destructors when they /are/ named. I suspect this was the cause of the original problem.
jrose-apple
force-pushed
the
mangle-typealiases-better
branch
from
d1efe11 to
09ac88b
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
:-( The builder is testing the correct commit but GitHub is showing them out of order. |
|
@swift-ci Please test Linux |
This can show up when trying to generate USRs for a document with errors in it. This isn't a great answer because the names it generates aren't unique (there may be more than one nameless entity with the same type), but it at least generates valid mangled names. When generating mangled names for purposes other than USRs, nameless entities are now checked for by an assertion.
I /think/ this can only come up in invalid code like
class Outer {
let _ = { class Inner {} }
}
but we still try to generate USRs for this. Just use the enclosing
context as the current context so that we still generate /some/ valid
mangling.
- Allow them to use substitutions. - Consistently use 'a' as a mangling operator. - For generic typealiases, include the alias as context for any generic parameters. Typealiases don't show up in symbol names, which always refer to canonical types, but they are mangled for debug info and for USRs (unique identifiers used by SourceKit), so it's good to get this right.
jrose-apple
force-pushed
the
mangle-typealiases-better
branch
from
09ac88b to
2d84981
Compare
|
Previous full-test passed. Still waiting on review from a SourceKit person. (Erik and I have been talking through chat.) @swift-ci Please smoke test |
|
Changes LGTM. |
6 participants
Typealiases don't show up in symbol names, which always refer to canonical types, but they are mangled for debug info and for USRs (unique identifiers used by SourceKit), so it's good to get this right.