Skip to content

[IDE] Fix type reconstructions for PrivateDeclName. #15126

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 12, 2018
Merged

[IDE] Fix type reconstructions for PrivateDeclName. #15126

merged 1 commit into from
Mar 12, 2018

Conversation

dcci
Copy link
Member

@dcci dcci commented Mar 9, 2018

Without this fix, the type reconstruction logic chokes on
demangled subtree of the kind:

kind=DeclContext
kind=Function
kind=Module, text="swift_ide_test"
kind=PrivateDeclName
kind=Identifier, text="_0830B395847924C73A6666B87EFD2ADF"
kind=Identifier, text="patatino"

Fixes the compiler part of rdar://problem/38248403

@dcci dcci requested review from slavapestov, rudkx and DougGregor March 9, 2018 21:45
@dcci
Copy link
Member Author

dcci commented Mar 9, 2018

@swift-ci please test

@dcci
Copy link
Member Author

dcci commented Mar 9, 2018 •
edited
Loading

More like

        kind=DeclContext
          kind=Function
            kind=Module, text="swift_ide_test"
            kind=Identifier, text="patatino"
            kind=LabelList
            kind=Type

It's a shame the indentation levels got lost in the comment.

@dcci dcci force-pushed the typereconstr-fixup-private branch from e959839 to feceee2 Compare March 9, 2018 22:00
@dcci
Copy link
Member Author

dcci commented Mar 9, 2018

Update to have the correct fix.

@dcci
Copy link
Member Author

dcci commented Mar 9, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

swift-ci commented Mar 9, 2018

Build failed
Swift Test Linux Platform
Git Sha - feceee221468b787c19b33710cd3eb5ccb6db533

adrian-prantl
adrian-prantl reviewed Mar 9, 2018
View reviewed changes
test/IDE/rdar38248403.swift Outdated
@@ -0,0 +1,9 @@
// RUN: %target-swift-ide-test -reconstruct-type -source-filename %s | %FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you give this file a more descriptive name instead of the radar id?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact I would just put this test case in the reconstruct_type_from_mangled_name

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the other file.

lib/IDE/TypeReconstruction.cpp Outdated
"demangled identifier %s could not be found by name lookup",
id_node->getText().str().c_str());
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps add a comment what is being done here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the helper, so this should be fairly self-explanatory now. Please let me know if you want me to add something else.

slavapestov
slavapestov reviewed Mar 10, 2018
View reviewed changes
lib/IDE/TypeReconstruction.cpp Outdated
@@ -1186,6 +1186,37 @@ static void VisitNodeFunction(
VisitNode(ast, *pos, decl_scope_result);
break;

case Demangle::Node::Kind::PrivateDeclName: {
if (child->getNumChildren() != 2 || !child->getChild(0)->hasText() ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you combine this code with the existing function VisitNodePrivateDeclName()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@slavapestov
Copy link
Contributor

@dcci Do you want to also do local types in a follow on patch? It should be similar to private, in fact there's a VisitNodeLocalDeclName() function you can re-use.

@dcci
Copy link
Member Author

dcci commented Mar 12, 2018

@slavapestov yes, I don't mind taking a look at local types as well in a follow-up. Also, updating this patch in a sec.

@rudkx rudkx removed their request for review March 12, 2018 19:02
@dcci dcci force-pushed the typereconstr-fixup-private branch from feceee2 to e6e432d Compare March 12, 2018 20:30
@dcci
Copy link
Member Author

dcci commented Mar 12, 2018

Sorry, morning in meetings. Updated.

dcci
dcci commented Mar 12, 2018
View reviewed changes
Copy link
Member Author

@dcci dcci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed Reviewers comments.

lib/IDE/TypeReconstruction.cpp Outdated
@@ -1186,6 +1186,37 @@ static void VisitNodeFunction(
VisitNode(ast, *pos, decl_scope_result);
break;

case Demangle::Node::Kind::PrivateDeclName: {
if (child->getNumChildren() != 2 || !child->getChild(0)->hasText() ||
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

lib/IDE/TypeReconstruction.cpp Outdated
"demangled identifier %s could not be found by name lookup",
id_node->getText().str().c_str());
break;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the helper, so this should be fairly self-explanatory now. Please let me know if you want me to add something else.

test/IDE/rdar38248403.swift Outdated
@@ -0,0 +1,9 @@
// RUN: %target-swift-ide-test -reconstruct-type -source-filename %s | %FileCheck %s
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the other file.

@dcci
Copy link
Member Author

dcci commented Mar 12, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - feceee221468b787c19b33710cd3eb5ccb6db533

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - feceee221468b787c19b33710cd3eb5ccb6db533

@dcci dcci force-pushed the typereconstr-fixup-private branch from e6e432d to 4746725 Compare March 12, 2018 20:34
@dcci
Copy link
Member Author

dcci commented Mar 12, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - e6e432d3d9329fcabbfa0e19a11368ba7e0e055e

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - e6e432d3d9329fcabbfa0e19a11368ba7e0e055e

@dcci
Copy link
Member Author

dcci commented Mar 12, 2018

@swift-ci please test

[IDE] Fix type reconstructions for PrivateDeclName.
74618cb 
Without this fix, the type reconstruction logic chokes on
demangled subtree of the kind:

kind=DeclContext
  kind=Function
    kind=Module, text="swift_ide_test"
    kind=PrivateDeclName
      kind=Identifier, text="_0830B395847924C73A6666B87EFD2ADF"
      kind=Identifier, text="patatino"

Fixes the compiler part of <rdar://problem/38248403>
@dcci dcci force-pushed the typereconstr-fixup-private branch from 4746725 to 74618cb Compare March 12, 2018 20:45
@dcci
Copy link
Member Author

dcci commented Mar 12, 2018

Updated tests

@dcci
Copy link
Member Author

dcci commented Mar 12, 2018

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 47467253584e998b40e043abc0f7aeabed1a28b6

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 47467253584e998b40e043abc0f7aeabed1a28b6

@vedantk
Copy link
Contributor

vedantk commented Mar 12, 2018

@swift-ci clean smoke test

@vedantk
Copy link
Contributor

vedantk commented Mar 12, 2018

@swift-ci clean

@dcci
Copy link
Member Author

dcci commented Mar 12, 2018 

16:08:54 CMake Error at CMakeLists.txt:90 (message):
16:08:54   LLDB test compilers not specified.  Tests will not run

I'm confident this is unrelated to my changes.

@dcci
Copy link
Member Author

dcci commented Mar 12, 2018

@swift-ci please clean smoke test

@dcci dcci merged commit 04ea4cc into swiftlang:master Mar 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slavapestov slavapestov slavapestov left review comments

@adrian-prantl adrian-prantl adrian-prantl left review comments

@DougGregor DougGregor Awaiting requested review from DougGregor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dcci @swift-ci @slavapestov @vedantk @adrian-prantl