Skip to content

Fix subscript reconstruction #15096

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

Merged
merged 2 commits into from
Mar 9, 2018
Merged

Fix subscript reconstruction #15096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0018173
IDE: Fix type reconstruction for subscripts
slavapestov Mar 8, 2018
0e637f4
IDE: Support for reconstruction of generic parameter types
slavapestov Mar 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 95 additions & 26 deletions lib/IDE/TypeReconstruction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,15 @@ static void VisitNodeDestructor(
}
}

static Demangle::NodePointer DropGenericSignature(
Demangle::NodePointer cur_node) {
if (cur_node->getKind() != Demangle::Node::Kind::DependentGenericType)
return nullptr;
if (cur_node->getChild(0) == nullptr)
return nullptr;
return cur_node->getFirstChild();
}

static void VisitNodeDeclContext(
ASTContext *ast,
Demangle::NodePointer cur_node, VisitNodeResult &result) {
Expand Down Expand Up @@ -979,14 +988,10 @@ static void VisitNodeDeclContext(
if (generics->getChild(0) == nullptr)
break;
generics = generics->getFirstChild();
if (generics->getKind() != Demangle::Node::Kind::DependentGenericType)
generics = DropGenericSignature(generics);
if (generics == nullptr)
break;
if (generics->getChild(0) == nullptr)
break;
generics = generics->getFirstChild();
// if (generics->getKind() !=
// Demangle::Node::Kind::ArchetypeList)
// break;

AbstractFunctionDecl *func_decl = nullptr;
for (Decl *decl : found_decls._decls) {
func_decl = dyn_cast<AbstractFunctionDecl>(decl);
Expand Down Expand Up @@ -1254,6 +1259,28 @@ static void VisitNodeFunction(
}
}

do {
if (cur_node->getKind() == Demangle::Node::Kind::Subscript) {
FindNamedDecls(ast, DeclBaseName::createSubscript(),
decl_scope_result);
if (decl_scope_result._decls.empty()) {
result._error = stringWithFormat(
"subscript identifier could not be found by name lookup");
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

If we get to this point do we really want to continue on to the code at line 1303?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above

}
std::copy(decl_scope_result._decls.begin(),
decl_scope_result._decls.end(),
back_inserter(identifier_result._decls));
std::copy(decl_scope_result._types.begin(),
decl_scope_result._types.end(),
back_inserter(identifier_result._types));
identifier_result._module = decl_scope_result._module;
if (decl_scope_result._decls.size() == 1)
found_univocous = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only used to break out of the earlier loop, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m just cargo culting the rest of the code in this function. Not sure I want to spend much time cleaning it up since most of it is going away soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case cargo-cult away, sir!

break;
}
} while (0);

// if (node_kind == Demangle::Node::Kind::Allocator)
// {
// // For allocators we don't have an identifier for
Expand Down Expand Up @@ -1416,6 +1443,8 @@ static void VisitNodeSetterGetter(
Demangle::NodePointer cur_node, VisitNodeResult &result) {
VisitNodeResult decl_ctx_result;
std::string identifier;
std::vector<StringRef> labels;

VisitNodeResult type_result;

assert(cur_node->getNumChildren() == 1 &&
Expand All @@ -1440,9 +1469,23 @@ static void VisitNodeSetterGetter(
case Demangle::Node::Kind::Identifier:
identifier.assign((*pos)->getText());
break;
case Demangle::Node::Kind::Type:
VisitNode(ast, *pos, type_result);
case Demangle::Node::Kind::Type: {
auto type = (*pos)->getFirstChild();
if (DropGenericSignature(type))
type = type->getChild(1);
VisitNode(ast, type, type_result);
break;
}
case Demangle::Node::Kind::LabelList: {
for (const auto &label : **pos) {
if (label->getKind() == Demangle::Node::Kind::FirstElementMarker)
labels.push_back(StringRef());
else {
labels.push_back(label->getText());
}
}
break;
}
default:
result._error =
stringWithFormat("%s encountered in generic type children",
Expand All @@ -1451,6 +1494,11 @@ static void VisitNodeSetterGetter(
}
}

if (!type_result.HasSingleType()) {
result._error = "bad type";
Copy link
Member

Choose a reason for hiding this comment

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

Do we have a testcase for this case/diagnostic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. These are invalid cases that should never come up (but maybe if your swiftmodule doesn’t match your debuginfo or something? I don’t know)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should worry too much (if we'll ever run across a case where this happens, we can consider adding a test), in particular considering that, as you say, this code might go away in the next while.

return;
}

if (referenced_node->getKind() == Demangle::Node::Kind::Subscript) {
// Since there can be many subscripts for the same nominal type, we need to
// find the one matching the specified type.
Expand All @@ -1467,9 +1515,6 @@ static void VisitNodeSetterGetter(
const AnyFunctionType *type_func =
type_result._types.front()->getAs<AnyFunctionType>();

Type type_result_type = type_func->getResult();
Type type_input_type = type_func->getInput();

FuncDecl *identifier_func = nullptr;

for (size_t i = 0; i < num_decls; i++) {
Expand All @@ -1494,24 +1539,24 @@ static void VisitNodeSetterGetter(
break;
}

if (identifier_func && identifier_func->getInterfaceType()) {
const AnyFunctionType *identifier_func_type =
identifier_func->getInterfaceType()->getAs<AnyFunctionType>();
if (identifier_func_type) {
if (identifier_func &&
subscript_decl->getGetter() &&
subscript_decl->getGetter()->getInterfaceType()) {
auto subscript_type =
subscript_decl->getGetter()->getInterfaceType()->getAs<AnyFunctionType>();

if (subscript_type) {
// Swift function types are formally functions that take the class
// and return the method,
// we have to strip off the first level of function call to compare
// against the type
// from the demangled name.
const AnyFunctionType *identifier_uncurried_result =
identifier_func_type->getResult()->getAs<AnyFunctionType>();
if (identifier_uncurried_result) {
Type identifier_result_type =
identifier_uncurried_result->getResult();
Type identifier_input_type =
identifier_uncurried_result->getInput();
if (identifier_result_type->isEqual(type_result_type) &&
identifier_input_type->isEqual(type_input_type)) {
auto subscript_uncurried_result =
subscript_type->getResult()->getAs<AnyFunctionType>();
if (subscript_uncurried_result) {
if (CompareFunctionTypes(type_func,
subscript_uncurried_result,
labels)) {
break;
}
}
Expand Down Expand Up @@ -2061,6 +2106,25 @@ VisitNodeWeak(ASTContext *ast,
}
}

static void
VisitNodeGenericParam(ASTContext *ast,
Demangle::NodePointer cur_node,
VisitNodeResult &result) {
if (cur_node->getNumChildren() == 2) {
auto first = cur_node->getChild(0);
auto second = cur_node->getChild(1);
if (first->getKind() == Demangle::Node::Kind::Index &&
second->getKind() == Demangle::Node::Kind::Index) {
result._types.push_back(
GenericTypeParamType::get(first->getIndex(),
second->getIndex(), *ast));
return;
}
}

result._error = "bad generic param type";
}

static void VisitFirstChildNode(
ASTContext *ast,
Demangle::NodePointer cur_node, VisitNodeResult &result) {
Expand Down Expand Up @@ -2159,7 +2223,8 @@ static void VisitNode(

case Demangle::Node::Kind::Function:
case Demangle::Node::Kind::Allocator:
case Demangle::Node::Kind::Variable: // Out of order on purpose
case Demangle::Node::Kind::Variable:
case Demangle::Node::Kind::Subscript: // Out of order on purpose
VisitNodeFunction(ast, node, result);
break;

Expand Down Expand Up @@ -2229,6 +2294,10 @@ static void VisitNode(
VisitNodeWeak(ast, node, result);
break;

case Demangle::Node::Kind::DependentGenericParamType:
VisitNodeGenericParam(ast, node, result);
break;

case Demangle::Node::Kind::LocalDeclName:
case Demangle::Node::Kind::Identifier:
case Demangle::Node::Kind::PrivateDeclName:
Expand Down
36 changes: 36 additions & 0 deletions test/IDE/reconstruct_type_from_mangled_name.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,3 +270,39 @@ fileprivate struct VeryPrivateData {}
// FIXME
// CHECK: decl: FAILURE for 'privateFunction'
fileprivate func privateFunction(_ d: VeryPrivateData) {}

struct HasSubscript {
// CHECK: decl: subscript(t: Int) -> Int { get set }
subscript(_ t: Int) -> Int {
// CHECK: decl: get {} for '' usr=s:14swift_ide_test12HasSubscriptVyS2icig
get {
return t
}
// CHECK: decl: set {} for '' usr=s:14swift_ide_test12HasSubscriptVyS2icis
set {}
}
}

// FIXME
// CHECK: decl: FAILURE for 'T' usr=s:14swift_ide_test19HasGenericSubscriptV1Txmfp
struct HasGenericSubscript<T> {
// CHECK: subscript<U>(t: T) -> U { get set } for 'subscript' usr=s:14swift_ide_test19HasGenericSubscriptVyqd__xclui
// FIXME
// CHECK: decl: FAILURE for 'U'
// FIXME
// CHECK: decl: FAILURE for 't'
subscript<U>(_ t: T) -> U {

// CHECK: decl: get {} for '' usr=s:14swift_ide_test19HasGenericSubscriptVyqd__xcluig
// FIXME
// CHECK: dref: FAILURE for 't'
get {
return t as! U
}

// FIXME
// CHECK: dref: FAILURE for 'U'
// CHECK: decl: set {} for '' usr=s:14swift_ide_test19HasGenericSubscriptVyqd__xcluis
set {}
}
}