Skip to content

Fix type reconstruction edge-cases with @escaping function values #25420

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

Merged
merged 2 commits into from
Jun 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions docs/ABI/Mangling.rst
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ Types
FUNCTION-KIND ::= 'U' // uncurried function type (currently not used)
FUNCTION-KIND ::= 'K' // @auto_closure function type (noescape)
FUNCTION-KIND ::= 'B' // objc block function type
FUNCTION-KIND ::= 'L' // objc block function type (escaping) (DWARF only; otherwise use 'B')
FUNCTION-KIND ::= 'C' // C function pointer type
FUNCTION-KIND ::= 'A' // @auto_closure function type (escaping)
FUNCTION-KIND ::= 'E' // function type (noescape)
Expand Down
1 change: 1 addition & 0 deletions include/swift/Demangling/DemangleNodes.def
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ NODE(NonObjCAttribute)
NODE(Number)
NODE(ObjCAttribute)
NODE(ObjCBlock)
NODE(EscapingObjCBlock)
CONTEXT_NODE(OtherNominalType)
CONTEXT_NODE(OwningAddressor)
CONTEXT_NODE(OwningMutableAddressor)
Expand Down
7 changes: 5 additions & 2 deletions include/swift/Demangling/TypeDecoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,7 @@ class TypeDecoder {
auto index = Node->getChild(1)->getIndex();
return Builder.createGenericTypeParameterType(depth, index);
}
case NodeKind::EscapingObjCBlock:
case NodeKind::ObjCBlock:
case NodeKind::CFunctionPointer:
case NodeKind::ThinFunctionType:
Expand All @@ -498,7 +499,8 @@ class TypeDecoder {
return BuiltType();

FunctionTypeFlags flags;
if (Node->getKind() == NodeKind::ObjCBlock) {
if (Node->getKind() == NodeKind::ObjCBlock ||
Node->getKind() == NodeKind::EscapingObjCBlock) {
flags = flags.withConvention(FunctionMetadataConvention::Block);
} else if (Node->getKind() == NodeKind::CFunctionPointer) {
flags =
Expand All @@ -524,7 +526,8 @@ class TypeDecoder {
.withParameterFlags(hasParamFlags)
.withEscaping(
Node->getKind() == NodeKind::FunctionType ||
Node->getKind() == NodeKind::EscapingAutoClosureType);
Node->getKind() == NodeKind::EscapingAutoClosureType ||
Node->getKind() == NodeKind::EscapingObjCBlock);

auto result = decodeMangledType(Node->getChild(isThrow ? 2 : 1));
if (!result) return BuiltType();
Expand Down
12 changes: 8 additions & 4 deletions lib/AST/ASTDemangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -378,10 +378,14 @@ Type ASTBuilder::createFunctionType(

auto einfo = AnyFunctionType::ExtInfo(representation,
/*throws*/ flags.throws());
if (flags.isEscaping())
einfo = einfo.withNoEscape(false);
else
einfo = einfo.withNoEscape(true);

if (representation == FunctionTypeRepresentation::Swift ||
representation == FunctionTypeRepresentation::Block) {
if (flags.isEscaping())
einfo = einfo.withNoEscape(false);
else
einfo = einfo.withNoEscape(true);
}

// The result type must be materializable.
if (!output->isMaterializable()) return Type();
Expand Down
4 changes: 4 additions & 0 deletions lib/AST/ASTMangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1911,6 +1911,10 @@ void ASTMangler::appendFunctionType(AnyFunctionType *fn, bool isAutoClosure,
// changes to better support thin functions.
switch (fn->getRepresentation()) {
case AnyFunctionType::Representation::Block:
// We distinguish escaping and non-escaping blocks, but only in the DWARF
// mangling, because the ABI is already set.
if (!fn->isNoEscape() && DWARFMangling)
return appendOperator("XL");
return appendOperator("XB");
case AnyFunctionType::Representation::Thin:
return appendOperator("Xf");
Expand Down
2 changes: 2 additions & 0 deletions lib/Demangling/Demangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2709,6 +2709,8 @@ NodePointer Demangler::demangleSpecialType() {
return popFunctionType(Node::Kind::AutoClosureType);
case 'U':
return popFunctionType(Node::Kind::UncurriedFunctionType);
case 'L':
return popFunctionType(Node::Kind::EscapingObjCBlock);
case 'B':
return popFunctionType(Node::Kind::ObjCBlock);
case 'C':
Expand Down
6 changes: 6 additions & 0 deletions lib/Demangling/NodePrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ class NodePrinter {
case Node::Kind::Directness:
case Node::Kind::DynamicAttribute:
case Node::Kind::EscapingAutoClosureType:
case Node::Kind::EscapingObjCBlock:
case Node::Kind::NoEscapeFunctionType:
case Node::Kind::ExplicitClosure:
case Node::Kind::Extension:
Expand Down Expand Up @@ -1788,6 +1789,11 @@ NodePointer NodePrinter::print(NodePointer Node, bool asPrefixContext) {
printFunctionType(nullptr, Node);
return nullptr;
}
case Node::Kind::EscapingObjCBlock: {
Printer << "@escaping @convention(block) ";
printFunctionType(nullptr, Node);
return nullptr;
}
case Node::Kind::SILBoxType: {
Printer << "@box ";
NodePointer type = Node->getChild(0);
Expand Down
5 changes: 5 additions & 0 deletions lib/Demangling/OldRemangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,11 @@ void Remangler::mangleObjCBlock(Node *node) {
mangleChildNodes(node); // argument tuple, result type
}

void Remangler::mangleEscapingObjCBlock(Node *node) {
// We shouldn't ever be remangling anything with a DWARF-only mangling.
Buffer << "<escaping block type>";
}

void Remangler::mangleCFunctionPointer(Node *node) {
Buffer << 'c';
mangleChildNodes(node); // argument tuple, result type
Expand Down
5 changes: 5 additions & 0 deletions lib/Demangling/Remangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1646,6 +1646,11 @@ void Remangler::mangleObjCBlock(Node *node) {
Buffer << "XB";
}

void Remangler::mangleEscapingObjCBlock(Node *node) {
mangleChildNodesReversed(node);
Buffer << "XL";
}

void Remangler::mangleOwningAddressor(Node *node) {
mangleAbstractStorage(node->getFirstChild(), "lO");
}
Expand Down
36 changes: 6 additions & 30 deletions lib/IRGen/IRGenDebugInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -761,28 +761,6 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
return Size(size);
}

#ifndef NDEBUG
static bool areTypesReallyEqual(Type lhs, Type rhs) {
// Due to an oversight, escaping and non-escaping @convention(block)
// are mangled identically.
auto eraseEscapingBlock = [](Type t) -> Type {
return t.transform([](Type t) -> Type {
if (auto *fnType = t->getAs<FunctionType>()) {
if (fnType->getExtInfo().getRepresentation()
== FunctionTypeRepresentation::Block) {
return FunctionType::get(fnType->getParams(),
fnType->getResult(),
fnType->getExtInfo().withNoEscape(true));
}
}
return t;
});
};

return eraseEscapingBlock(lhs)->isEqual(eraseEscapingBlock(rhs));
}
#endif

StringRef getMangledName(DebugTypeInfo DbgTy) {
if (DbgTy.IsMetadataType)
return MetadataTypeDeclCache.find(DbgTy.getDecl()->getName().str())
Expand Down Expand Up @@ -825,14 +803,12 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
Ty->dump();
abort();
} else if (!Reconstructed->isEqual(Ty)) {
if (!areTypesReallyEqual(Reconstructed, Ty)) {
llvm::errs() << "Incorrect reconstructed type for " << Result << "\n";
llvm::errs() << "Original type:\n";
Ty->dump();
llvm::errs() << "Reconstructed type:\n";
Reconstructed->dump();
abort();
}
llvm::errs() << "Incorrect reconstructed type for " << Result << "\n";
llvm::errs() << "Original type:\n";
Ty->dump();
llvm::errs() << "Reconstructed type:\n";
Reconstructed->dump();
abort();
}
#endif
}
Expand Down
19 changes: 19 additions & 0 deletions test/TypeDecoder/structural_types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ do {
let escaping: (@escaping () -> ()) -> () = { _ in }
blackHole(escaping)
}

do {
let c: [@convention(c) () -> ()] = []
blackHole(c)
}

do {
let b: [(@escaping @convention(block) () -> (), @convention(block) () -> ()) -> ()] = []
blackHole(b)
}

// DEMANGLE: $syycD
// DEMANGLE: $sySSzcD
// DEMANGLE: $sySSncD
Expand All @@ -136,6 +147,8 @@ do {
// DEMANGLE: $sSim_Sf1xSitD
// DEMANGLE: $sSi1x_SfSim1ytD
// DEMANGLE: $syyyccD
// DEMANGLE: $sSayyyXCGD
// DEMANGLE: $sSayyyyXL_yyXBtcGD

// CHECK: () -> ()
// CHECK: (inout String) -> ()
Expand All @@ -153,6 +166,8 @@ do {
// CHECK: (Int.Type, x: Float, Int)
// CHECK: (x: Int, Float, y: Int.Type)
// CHECK: (@escaping () -> ()) -> ()
// CHECK: Array<@convention(c) () -> ()>
// CHECK: Array<(@escaping @convention(block) () -> (), @convention(block) () -> ()) -> ()>

// DEMANGLE: $sSimD
// DEMANGLE: $syycmD
Expand All @@ -171,6 +186,8 @@ do {
// DEMANGLE: $sSim_Sf1xSitmD
// DEMANGLE: $sSi1x_SfSim1ytmD
// DEMANGLE: $syyyccmD
// DEMANGLE: $sSayyyXCGmD
// DEMANGLE: $sSayyyyXL_yyXBtcGmD

// CHECK: Int.Type
// CHECK: ((inout String) -> ()).Type
Expand All @@ -188,3 +205,5 @@ do {
// CHECK: (Int.Type, x: Float, Int).Type
// CHECK: (x: Int, Float, y: Int.Type).Type
// CHECK: ((@escaping () -> ()) -> ()).Type
// CHECK: Array<@convention(c) () -> ()>.Type
// CHECK: Array<(@escaping @convention(block) () -> (), @convention(block) () -> ()) -> ()>.Type