Skip to content

swift-api-digester: diagnose the addition and removal of @objc, @_fixed_layout and @_frozen under ABI mode. #18739

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
Aug 16, 2018
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
3 changes: 3 additions & 0 deletions include/swift/IDE/DigesterEnums.def
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ NODE_ANNOTATION(NowThrowing)
NODE_ANNOTATION(NowMutating)
NODE_ANNOTATION(StaticChange)
NODE_ANNOTATION(OwnershipChange)
NODE_ANNOTATION(ChangeObjC)
NODE_ANNOTATION(ChangeFixedLayout)
NODE_ANNOTATION(ChangeFrozen)
NODE_ANNOTATION(RawTypeLeft)
NODE_ANNOTATION(RawTypeRight)

Expand Down
10 changes: 9 additions & 1 deletion test/api-digester/Inputs/cake1.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,12 @@ public struct Somestruct2 {

public class C4: OldType {
public func foo() {}
}
}

@objc
public class C5 {}

public struct C6 {}

@_frozen
public enum IceKind {}
9 changes: 8 additions & 1 deletion test/api-digester/Inputs/cake2.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,11 @@ public struct NSSomestruct2 {
public static func foo1(_ a : C3) {}
}

public class C4: NewType {}
public class C4: NewType {}

public class C5 {}

@_fixed_layout
public struct C6 {}

public enum IceKind {}
27 changes: 27 additions & 0 deletions test/api-digester/Outputs/Cake-abi.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@

/* RawRepresentable Changes */

/* Removed Decls */
cake1: Constructor Somestruct2.init(_:) has been removed
cake1: Func C4.foo() has been removed

/* Moved Decls */

/* Renamed Decls */
cake1: Struct Somestruct2 has been renamed to Struct NSSomestruct2
cake1: Func S1.foo5(x:y:) has been renamed to Func S1.foo5(x:y:z:)

/* Type Changes */
cake1: Constructor S1.init(_:) has parameter 0 type change from Int to Double
cake1: Func C1.foo2(_:) has parameter 0 type change from Int to () -> ()
cake1: Func Somestruct2.foo1(_:) has parameter 0 type change from C3 to C1

/* Decl Attribute changes */
cake1: Enum IceKind is now without @_frozen
cake1: Struct C6 is now with @_fixed_layout
cake1: Class C5 is now without @objc
cake1: Var C1.CIIns1 changes from weak to strong
cake1: Var C1.CIIns2 changes from strong to weak
cake1: Func C1.foo1() is now not static
cake1: Func S1.foo1() is now mutating
cake1: Func S1.foo3() is now static
2 changes: 1 addition & 1 deletion test/api-digester/Outputs/Cake.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ cake1: Func C1.foo2(_:) has parameter 0 type change from Int to () -> ()
cake1: Var C1.CIIns1 changes from weak to strong
cake1: Var C1.CIIns2 changes from strong to weak
cake1: Func C1.foo1() is now not static
cake1: Func S1.foo3() is now static
cake1: Func S1.foo1() is now mutating
cake1: Func S1.foo3() is now static
5 changes: 5 additions & 0 deletions test/api-digester/compare-dump.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,8 @@
// RUN: %api-digester -dump-sdk -module cake2 -o %t.dump2.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod -I %S/Inputs/APINotesRight
// RUN: %api-digester -diagnose-sdk -print-module --input-paths %t.dump1.json -input-paths %t.dump2.json > %t.result
// RUN: diff -u %S/Outputs/Cake.txt %t.result

// RUN: %api-digester -dump-sdk -module cake1 -o %t.dump1.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod -I %S/Inputs/APINotesLeft -abi
// RUN: %api-digester -dump-sdk -module cake2 -o %t.dump2.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod -I %S/Inputs/APINotesRight -abi
// RUN: %api-digester -diagnose-sdk -print-module --input-paths %t.dump1.json -input-paths %t.dump2.json -abi > %t.result
// RUN: diff -u %S/Outputs/Cake-abi.txt %t.result
100 changes: 85 additions & 15 deletions tools/swift-api-digester/swift-api-digester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,16 +252,43 @@ class UpdatedNodesMap : public MatchedNodeListener {
}
};

// Describing some attributes with ABI impact. The addition or removal of these
// attributes is considerred ABI-breaking.
struct ABIAttributeInfo {
const DeclAttrKind Kind;
const NodeAnnotation Annotation;
const StringRef Content;
};

class SDKContext {
bool ABI;
llvm::StringSet<> TextData;
llvm::BumpPtrAllocator Allocator;
UpdatedNodesMap UpdateMap;
NodeMap TypeAliasUpdateMap;
NodeMap RevertTypeAliasUpdateMap;
TypeMemberDiffVector TypeMemberDiffs;

bool ABI;
std::vector<ABIAttributeInfo> ABIAttrs;

static StringRef getAttrName(DeclAttrKind Kind) {
switch (Kind) {
#define DECL_ATTR(NAME, CLASS, ...) case DAK_##CLASS: return "@"#NAME;
#include "swift/AST/Attr.def"
case DAK_Count:
llvm_unreachable("unrecognized attribute kind.");
}
}

public:
SDKContext(bool ABI): ABI(ABI) {}
SDKContext(bool ABI): ABI(ABI) {
#define ADD(NAME) ABIAttrs.push_back({DeclAttrKind::DAK_##NAME, \
NodeAnnotation::Change##NAME, getAttrName(DeclAttrKind::DAK_##NAME)});
ADD(ObjC)
ADD(FixedLayout)
ADD(Frozen)
#undef ADD
}
llvm::BumpPtrAllocator &allocator() {
return Allocator;
}
Expand All @@ -281,6 +308,7 @@ class SDKContext {
return TypeMemberDiffs;
}
bool checkingABI() const { return ABI; }
ArrayRef<ABIAttributeInfo> getABIAttributeInfo() const { return ABIAttrs; }
};

// A node matcher will traverse two trees of SDKNode and find matched nodes
Expand Down Expand Up @@ -446,7 +474,6 @@ class SDKNodeDecl : public SDKNode {
bool IsStatic;
bool IsDeprecated;
uint8_t ReferenceOwnership;
bool hasDeclAttribute(DeclAttrKind DAKind) const;
StringRef GenericSig;

protected:
Expand All @@ -464,6 +491,7 @@ class SDKNodeDecl : public SDKNode {
StringRef getModuleName() const {return ModuleName;}
StringRef getHeaderName() const;
ArrayRef<DeclAttrKind> getDeclAttributes() const;
bool hasAttributeChange(const SDKNodeDecl &Another) const;
swift::ReferenceOwnership getReferenceOwnership() const {
return swift::ReferenceOwnership(ReferenceOwnership);
}
Expand All @@ -474,7 +502,7 @@ class SDKNodeDecl : public SDKNode {
StringRef getFullyQualifiedName() const;
bool isSDKPrivate() const;
bool isDeprecated() const { return IsDeprecated; };
bool hasFixedLayout() const;
bool hasDeclAttribute(DeclAttrKind DAKind) const;
bool isStatic() const { return IsStatic; };
StringRef getGenericSignature() const { return GenericSig; }
};
Expand Down Expand Up @@ -761,10 +789,6 @@ SDKNode *SDKNodeRoot::getInstance(SDKContext &Ctx) {
return Info.createSDKNode(SDKNodeKind::Root);
}

bool SDKNodeDecl::hasFixedLayout() const {
return hasDeclAttribute(DeclAttrKind::DAK_FixedLayout);
}

bool SDKNodeDecl::isSDKPrivate() const {
if (getName().startswith("__"))
return true;
Expand Down Expand Up @@ -818,6 +842,16 @@ ArrayRef<DeclAttrKind> SDKNodeDecl::getDeclAttributes() const {
return llvm::makeArrayRef(DeclAttributes.data(), DeclAttributes.size());
}

bool SDKNodeDecl::hasAttributeChange(const SDKNodeDecl &Another) const {
if (getDeclAttributes().size() != Another.getDeclAttributes().size())
return true;
for (auto K: getDeclAttributes()) {
if (!Another.hasDeclAttribute(K))
return true;
}
return false;
}

SDKNodeDecl *SDKNodeType::getClosestParentDecl() const {
auto *Result = getParent();
for (; !isa<SDKNodeDecl>(Result); Result = Result->getParent());
Expand Down Expand Up @@ -1175,6 +1209,10 @@ bool SDKNode::operator==(const SDKNode &Other) const {
return false;
if (Left->getReferenceOwnership() != Right->getReferenceOwnership())
return false;
if (Left->hasAttributeChange(*Right))
return false;
if (Left->getGenericSignature() != Right->getGenericSignature())
return false;
LLVM_FALLTHROUGH;
}
case SDKNodeKind::Root: {
Expand Down Expand Up @@ -2454,13 +2492,19 @@ static bool isOwnershipEquivalent(ReferenceOwnership Left,

static void detectDeclChange(NodePtr L, NodePtr R) {
assert(L->getKind() == R->getKind());
auto &Ctx = L->getSDKContext();
if (auto LD = dyn_cast<SDKNodeDecl>(L)) {
auto *RD = R->getAs<SDKNodeDecl>();
if (LD->isStatic() ^ RD->isStatic())
L->annotate(NodeAnnotation::StaticChange);
if (!isOwnershipEquivalent(LD->getReferenceOwnership(),
RD->getReferenceOwnership()))
L->annotate(NodeAnnotation::OwnershipChange);
// Check if some attributes with ABI-impact have been added/removed.
for (auto &Info: Ctx.getABIAttributeInfo()) {
if (LD->hasDeclAttribute(Info.Kind) != RD->hasDeclAttribute(Info.Kind))
L->annotate(Info.Annotation);
}
detectRename(L, R);
}
}
Expand Down Expand Up @@ -3266,6 +3310,8 @@ class DiagnosisEmitter : public SDKNodeVisitor {
llvm::outs() << " */\n";
removeRedundantAndSort(Diags);
std::for_each(Diags.begin(), Diags.end(), [](T &Diag) {
if (Diag.isABISpecific() && !options::Abi)
return;
Diag.outputModule();
Diag.output();
});
Expand All @@ -3275,12 +3321,15 @@ class DiagnosisEmitter : public SDKNodeVisitor {
struct MetaInfo {
StringRef ModuleName;
StringRef HeaderName;
MetaInfo(const SDKNodeDecl *Node):
ModuleName(Node->getModuleName()), HeaderName(Node->getHeaderName()) {}
bool IsABISpecific;
MetaInfo(const SDKNodeDecl *Node, bool IsABISpecific = false):
ModuleName(Node->getModuleName()), HeaderName(Node->getHeaderName()),
IsABISpecific(IsABISpecific) {}
};

struct DiagBase {
class DiagBase {
MetaInfo Info;
public:
DiagBase(MetaInfo Info): Info(Info) {}
virtual ~DiagBase() = default;
void outputModule() const {
Expand All @@ -3292,6 +3341,7 @@ class DiagnosisEmitter : public SDKNodeVisitor {
}
}
virtual void output() const = 0;
bool isABISpecific() const { return Info.IsABISpecific; }
};

struct RemovedDeclDiag: public DiagBase {
Expand Down Expand Up @@ -3513,7 +3563,9 @@ void DiagnosisEmitter::DeclTypeChangeDiag::output() const {
bool DiagnosisEmitter::DeclAttrDiag::operator<(DeclAttrDiag Other) const {
if (Kind != Other.Kind)
return Kind < Other.Kind;
return DeclName.compare_lower(Other.DeclName);
if (DeclName != Other.DeclName)
return DeclName.compare(Other.DeclName) < 0;
return AttrAfter.compare(Other.AttrAfter) < 0;
}

void DiagnosisEmitter::DeclAttrDiag::output() const {
Expand All @@ -3535,9 +3587,9 @@ void DiagnosisEmitter::diagnosis(NodePtr LeftRoot, NodePtr RightRoot,
void DiagnosisEmitter::handle(const SDKNodeDecl *Node, NodeAnnotation Anno) {
assert(Node->isAnnotatedAs(Anno));
auto &Ctx = Node->getSDKContext();
MetaInfo ScreenInfo(Node);
switch(Anno) {
case NodeAnnotation::Removed: {
MetaInfo ScreenInfo(Node, false);
// If we can find a type alias decl with the same name of this type, we
// consider the type is not removed.
if (findTypeAliasDecl(Node))
Expand Down Expand Up @@ -3599,6 +3651,7 @@ void DiagnosisEmitter::handle(const SDKNodeDecl *Node, NodeAnnotation Anno) {
return;
}
case NodeAnnotation::Rename: {
MetaInfo ScreenInfo(Node, false);
auto *Count = UpdateMap.findUpdateCounterpart(Node)->getAs<SDKNodeDecl>();
RenamedDecls.Diags.emplace_back(ScreenInfo,
Node->getDeclKind(), Count->getDeclKind(),
Expand All @@ -3607,27 +3660,31 @@ void DiagnosisEmitter::handle(const SDKNodeDecl *Node, NodeAnnotation Anno) {
return;
}
case NodeAnnotation::NowMutating: {
MetaInfo ScreenInfo(Node, false);
AttrChangedDecls.Diags.emplace_back(ScreenInfo,
Node->getDeclKind(),
Node->getFullyQualifiedName(),
Ctx.buffer("mutating"));
return;
}
case NodeAnnotation::NowThrowing: {
MetaInfo ScreenInfo(Node, false);
AttrChangedDecls.Diags.emplace_back(ScreenInfo,
Node->getDeclKind(),
Node->getFullyQualifiedName(),
Ctx.buffer("throwing"));
return;
}
case NodeAnnotation::StaticChange: {
MetaInfo ScreenInfo(Node, false);
AttrChangedDecls.Diags.emplace_back(ScreenInfo,
Node->getDeclKind(),
Node->getFullyQualifiedName(),
Ctx.buffer(Node->isStatic() ? "not static" : "static"));
return;
}
case NodeAnnotation::OwnershipChange: {
MetaInfo ScreenInfo(Node, false);
auto getOwnershipDescription = [&](swift::ReferenceOwnership O) {
if (O == ReferenceOwnership::Strong)
return Ctx.buffer("strong");
Expand All @@ -3640,9 +3697,22 @@ void DiagnosisEmitter::handle(const SDKNodeDecl *Node, NodeAnnotation Anno) {
getOwnershipDescription(Count->getReferenceOwnership()));
return;
}
default:
default: {
// Diagnose the addition/removal of attributes with ABI impact.
auto Infos = Ctx.getABIAttributeInfo();
auto It = std::find_if(Infos.begin(), Infos.end(),
[&](const ABIAttributeInfo &I) { return I.Annotation == Anno; });
if (It == Infos.end())
return;
MetaInfo ScreenInfo(Node, true);
auto Desc = Node->hasDeclAttribute(It->Kind) ?
Ctx.buffer((llvm::Twine("without ") + It->Content).str()):
Ctx.buffer((llvm::Twine("with ") + It->Content).str());
AttrChangedDecls.Diags.emplace_back(ScreenInfo, Node->getDeclKind(),
Node->getFullyQualifiedName(), Desc);
return;
}
}
}

void DiagnosisEmitter::visitDecl(SDKNodeDecl *Node) {
Expand Down Expand Up @@ -3692,7 +3762,7 @@ void DiagnosisEmitter::visit(NodePtr Node) {
}
}

typedef std::vector<NoEscapeFuncParam> NoEscapeFuncParamVector;
typedef std::vector<NoEscapeFuncParam> NoEscapeFuncParamVector;

class NoEscapingFuncEmitter : public SDKNodeVisitor {
NoEscapeFuncParamVector &AllItems;
Expand Down