From d7f5dc4b28f408af291b875303c883549f9aec94 Mon Sep 17 00:00:00 2001 From: Xi Ge Date: Mon, 23 Aug 2021 14:21:08 -0700 Subject: [PATCH] ABIChecker: diagnose mangled name changes Moving generic constraint from extension to its member or vice versa may change the mangled name of the member without changing the generic signature, thus introducing ABI breakages. This change teaches the ABI checker to diagnose USR (mangled name) changes to cover such cases. rdar://78276290 --- include/swift/AST/DiagnosticsModuleDiffer.def | 2 ++ include/swift/AST/USRGeneration.h | 3 +++ lib/APIDigester/ModuleAnalyzerNodes.cpp | 9 ++++++++ lib/APIDigester/ModuleDiagsConsumer.cpp | 1 + lib/AST/USRGeneration.cpp | 13 ++++++++++++ test/api-digester/Outputs/Cake-abi.txt | 10 +++++++++ test/api-digester/diff-demangled-name.swift | 21 +++++++++++++++++++ 7 files changed, 59 insertions(+) create mode 100644 test/api-digester/diff-demangled-name.swift diff --git a/include/swift/AST/DiagnosticsModuleDiffer.def b/include/swift/AST/DiagnosticsModuleDiffer.def index 443117a8d6afa..b8d7276a150d5 100644 --- a/include/swift/AST/DiagnosticsModuleDiffer.def +++ b/include/swift/AST/DiagnosticsModuleDiffer.def @@ -88,6 +88,8 @@ ERROR(not_inheriting_convenience_inits,APIDigesterBreakage,"%0 no longer inherit ERROR(enum_case_added,APIDigesterBreakage,"%0 has been added as a new enum case", (StringRef)) +ERROR(demangled_name_changed,APIDigesterBreakage,"%0 has mangled name changing from '%1' to '%2'", (StringRef, StringRef, StringRef)) + WARNING(cannot_read_allowlist,none,"cannot read breakage allowlist at '%0'", (StringRef)) #define UNDEFINE_DIAGNOSTIC_MACROS diff --git a/include/swift/AST/USRGeneration.h b/include/swift/AST/USRGeneration.h index 7b96c4d3e3c27..4d4f116c0154c 100644 --- a/include/swift/AST/USRGeneration.h +++ b/include/swift/AST/USRGeneration.h @@ -61,6 +61,9 @@ bool printExtensionUSR(const ExtensionDecl *ED, raw_ostream &OS); /// \returns true if it failed, false on success. bool printDeclUSR(const Decl *D, raw_ostream &OS); +/// Demangle a mangle-name-based USR to a human readable name. +std::string demangleUSR(StringRef mangled); + } // namespace ide } // namespace swift diff --git a/lib/APIDigester/ModuleAnalyzerNodes.cpp b/lib/APIDigester/ModuleAnalyzerNodes.cpp index 81f20ca5cc880..eaebc11e43bc4 100644 --- a/lib/APIDigester/ModuleAnalyzerNodes.cpp +++ b/lib/APIDigester/ModuleAnalyzerNodes.cpp @@ -944,6 +944,8 @@ static bool isSDKNodeEqual(SDKContext &Ctx, const SDKNode &L, const SDKNode &R) if (Left->getFixedBinaryOrder() != Right->getFixedBinaryOrder()) return false; } + if (Left->getUsr() != Right->getUsr()) + return false; LLVM_FALLTHROUGH; } case SDKNodeKind::Conformance: @@ -2551,6 +2553,13 @@ void swift::ide::api::SDKNodeDecl::diagnose(SDKNode *Right) { emitDiag(Loc, diag::decl_reorder, getFixedBinaryOrder(), RD->getFixedBinaryOrder()); } + if (getUsr() != RD->getUsr()) { + auto left = demangleUSR(getUsr()); + auto right = demangleUSR(RD->getUsr()); + if (left != right) { + emitDiag(Loc, diag::demangled_name_changed, left, right); + } + } } } diff --git a/lib/APIDigester/ModuleDiagsConsumer.cpp b/lib/APIDigester/ModuleDiagsConsumer.cpp index 044c9d05926d9..c73fd148d5947 100644 --- a/lib/APIDigester/ModuleDiagsConsumer.cpp +++ b/lib/APIDigester/ModuleDiagsConsumer.cpp @@ -54,6 +54,7 @@ static StringRef getCategoryName(uint32_t ID) { case LocalDiagID::raw_type_change: return "/* RawRepresentable Changes */"; case LocalDiagID::generic_sig_change: + case LocalDiagID::demangled_name_changed: return "/* Generic Signature Changes */"; case LocalDiagID::enum_case_added: case LocalDiagID::decl_added: diff --git a/lib/AST/USRGeneration.cpp b/lib/AST/USRGeneration.cpp index c3fe2322c21c9..5a02175895e8f 100644 --- a/lib/AST/USRGeneration.cpp +++ b/lib/AST/USRGeneration.cpp @@ -19,6 +19,7 @@ #include "swift/AST/SwiftNameTranslation.h" #include "swift/AST/TypeCheckRequests.h" #include "swift/AST/USRGeneration.h" +#include "swift/Demangling/Demangler.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/raw_ostream.h" @@ -255,6 +256,18 @@ swift::USRGenerationRequest::evaluate(Evaluator &evaluator, return NewMangler.mangleDeclAsUSR(D, getUSRSpacePrefix()); } +std::string ide::demangleUSR(StringRef mangled) { + if (mangled.startswith(getUSRSpacePrefix())) { + mangled = mangled.substr(getUSRSpacePrefix().size()); + } + SmallString<128> buffer; + buffer += "$s"; + buffer += mangled; + mangled = buffer.str(); + Demangler Dem; + return nodeToString(Dem.demangleSymbol(mangled)); +} + std::string swift::MangleLocalTypeDeclRequest::evaluate(Evaluator &evaluator, const TypeDecl *D) const { diff --git a/test/api-digester/Outputs/Cake-abi.txt b/test/api-digester/Outputs/Cake-abi.txt index a0ed1dcc8ac50..7010f162d7149 100644 --- a/test/api-digester/Outputs/Cake-abi.txt +++ b/test/api-digester/Outputs/Cake-abi.txt @@ -1,7 +1,17 @@ /* Generic Signature Changes */ +cake: Constructor S1.init(_:) has mangled name changing from 'cake.S1.init(Swift.Int) -> cake.S1' to 'cake.S1.init(Swift.Double) -> cake.S1' +cake: Func C1.foo1() has mangled name changing from 'static cake.C1.foo1() -> ()' to 'cake.C1.foo1() -> ()' +cake: Func C1.foo2(_:) has mangled name changing from 'cake.C1.foo2(Swift.Int) -> ()' to 'cake.C1.foo2(() -> ()) -> ()' cake: Func P1.P1Constraint() has generic signature change from to +cake: Func P1.P1Constraint() has mangled name changing from '(extension in cake):cake.P1< where A: cake.P2>.P1Constraint() -> ()' to '(extension in cake):cake.P1.P1Constraint() -> ()' +cake: Func S1.foo3() has mangled name changing from 'cake.S1.foo3() -> ()' to 'static cake.S1.foo3() -> ()' +cake: Func S1.foo5(x:y:) has mangled name changing from 'cake.S1.foo5(x: Swift.Int, y: Swift.Int) -> ()' to 'cake.S1.foo5(x: Swift.Int, y: Swift.Int, z: Swift.Int) -> ()' +cake: Func Somestruct2.foo1(_:) has mangled name changing from 'static cake.Somestruct2.foo1(cake.C3) -> ()' to 'static cake.NSSomestruct2.foo1(cake.C1) -> ()' +cake: Func ownershipChange(_:_:) has mangled name changing from 'cake.ownershipChange(inout Swift.Int, __shared Swift.Int) -> ()' to 'cake.ownershipChange(Swift.Int, __owned Swift.Int) -> ()' +cake: Func returnFunctionTypeOwnershipChange() has mangled name changing from 'cake.returnFunctionTypeOwnershipChange() -> (cake.C1) -> ()' to 'cake.returnFunctionTypeOwnershipChange() -> (__owned cake.C1) -> ()' cake: Protocol P3 has generic signature change from to +cake: Struct Somestruct2 has mangled name changing from 'cake.Somestruct2' to 'cake.NSSomestruct2' /* RawRepresentable Changes */ diff --git a/test/api-digester/diff-demangled-name.swift b/test/api-digester/diff-demangled-name.swift new file mode 100644 index 0000000000000..3a5d8290508d4 --- /dev/null +++ b/test/api-digester/diff-demangled-name.swift @@ -0,0 +1,21 @@ +// RUN: %empty-directory(%t) +// RUN: echo "public func foo() {}" > %t/Foo.swift + +// RUN: echo "public protocol P { associatedtype A }" > %t/Foo-1.swift +// RUN: echo "public extension P { public func f() where A == Int {} }" >> %t/Foo-1.swift + +// RUN: echo "public protocol P { associatedtype A }" > %t/Foo-2.swift +// RUN: echo "public extension P where A == Int { public func f() {} }" >> %t/Foo-2.swift + +// RUN: %target-swift-frontend -emit-module %t/Foo-1.swift -module-name Foo -emit-module-interface-path %t/Foo1.swiftinterface +// RUN: %target-swift-frontend -emit-module %t/Foo-2.swift -module-name Foo -emit-module-interface-path %t/Foo2.swiftinterface + +// RUN: %target-swift-frontend -compile-module-from-interface %t/Foo1.swiftinterface -o %t/Foo1.swiftmodule -module-name Foo -emit-abi-descriptor-path %t/Foo1.json + +// RUN: %target-swift-frontend -compile-module-from-interface %t/Foo2.swiftinterface -o %t/Foo2.swiftmodule -module-name Foo -emit-abi-descriptor-path %t/Foo2.json + +// RUN: %api-digester -diagnose-sdk -print-module --input-paths %t/Foo1.json -input-paths %t/Foo2.json -abi -o %t/result.txt + +// RUN: %FileCheck %s < %t/result.txt + +// CHECK: Foo: Func P.f() has mangled name changing from '(extension in Foo):Foo.P.f< where A.A == Swift.Int>() -> ()' to '(extension in Foo):Foo.P< where A.A == Swift.Int>.f() -> ()'