Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Demangler] Don't truncate demangling of an associated type descriptor #26980

Merged
merged 5 commits into from Sep 3, 2019

Conversation

@theblixguy
Copy link
Collaborator

commented Aug 31, 2019

When demangling an associated type descriptor name like $s5Assoc3Kit5ProtoPTl, the associated type's name is printed, but information about the protocol it belongs to is not, even though it's available in the mangled name. When we have information about the protocol that the type belongs to, print that as well.

$ swift demangle -expand s5Assoc3Kit5ProtoPTl
Demangling for $s5Assoc3Kit5ProtoPTl
kind=Global
  kind=AssociatedTypeDescriptor
    kind=DependentAssociatedTypeRef
      kind=Identifier, text="Assoc"
      kind=Type
        kind=Protocol
          kind=Module, text="Kit"
          kind=Identifier, text="Proto"
$s5Assoc3Kit5ProtoPTl ---> associated type descriptor for Assoc

Resolves SR-11403.

[Demangler] When printing an associated type descriptor, print the fu…
…ll name of the associatedtype

When we have information about the protocol that the type belongs to, print that as well.

@theblixguy theblixguy requested a review from brentdax Aug 31, 2019

@theblixguy

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 31, 2019

@swift-ci please smoke test

lib/Demangling/NodePrinter.cpp Outdated Show resolved Hide resolved

@theblixguy theblixguy force-pushed the theblixguy:fix/SR-11403 branch from feef2da to 8b7c5fe Sep 1, 2019

@brentdax

This comment has been minimized.

Copy link
Collaborator

commented Sep 2, 2019

@swift-ci please smoke test

@brentdax
Copy link
Collaborator

left a comment

Nice and clean now!

@theblixguy

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 2, 2019

Hmm, it seems like we may not have the extra node in some cases, so we have to guard against that. Also, it seems like some of the tests failed because the demangled string was different due to this change.

Let me update the tests.

@theblixguy

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 2, 2019

@swift-ci please smoke test macOS platform

@theblixguy

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 2, 2019

@swift-ci please smoke test macOS platform

@theblixguy

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 3, 2019

I wish FileCheck would dump all the failures at once instead of one at a time :/

I’ve been having some issues with running tests locally (kernel panics on Catalina beta), but let me try to resolve them locally and then push my changes.

@theblixguy

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 3, 2019

@swift-ci please smoke test

@theblixguy theblixguy merged commit 7c3d709 into apple:master Sep 3, 2019

2 checks passed

Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform (smoke test)
Details

@theblixguy theblixguy deleted the theblixguy:fix/SR-11403 branch Sep 3, 2019

@brentdax

This comment has been minimized.

Copy link
Collaborator

commented Sep 3, 2019

@theblixguy You know, looking at the test cases you changed, I'm not sure this is the best way to print these things. For example, in:

_TtuRxs8Runciblexs8FungiblerFxwxPS_5Mince ---> <A where A: Swift.Runcible, A: Swift.Fungible>(A) -> A.Swift.Runcible.Mince

The return type might be better rendered as something like A.Mince (from Swift.Runcible). It's a shame we don't already have a syntax for this that we could crib from...

@theblixguy

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 3, 2019

I think we do something similar for extensions, for example: “A.Swift._IndexableBase.Index in (extension in a)”.

@brentdax

This comment has been minimized.

Copy link
Collaborator

commented Sep 3, 2019

It looks like we now add the Swift._IndexableBase in the middle after this PR, but we didn't before it. Or do you mean the in (extension in a) bit?

@theblixguy

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 3, 2019

Yeah, I meant the “in (extension in a)” bit.

@brentdax

This comment has been minimized.

Copy link
Collaborator

commented Sep 3, 2019

Right. I meant a syntax in Swift itself—for instance, if the language allowed you to say something like Runcible::Mince to distinguish it from a Mince associated type you got from some other protocol, that might give us an idea of how to spell this thing in a demangling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.