Skip to content

ABI/API checker: teach the tool to emit diagnostics to serialized source location for decls #27766

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 1 commit into from
Oct 18, 2019
Merged

ABI/API checker: teach the tool to emit diagnostics to serialized source location for decls #27766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions include/swift/Basic/SourceManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class SourceManager {
};
std::map<const char *, VirtualFile> VirtualFiles;
mutable std::pair<const char *, const VirtualFile*> CachedVFile = {nullptr, nullptr};

Optional<unsigned> findBufferContainingLocInternal(SourceLoc Loc) const;
public:
SourceManager(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS =
llvm::vfs::getRealFileSystem())
Expand Down Expand Up @@ -115,6 +117,9 @@ class SourceManager {
/// this routine always returns a valid buffer ID.
unsigned findBufferContainingLoc(SourceLoc Loc) const;

/// Whether the source location is pointing to any buffer owned by the SourceManager.
bool isOwning(SourceLoc Loc) const;

/// Adds a memory buffer to the SourceManager, taking ownership of it.
unsigned addNewSourceBuffer(std::unique_ptr<llvm::MemoryBuffer> Buffer);

Expand Down
14 changes: 13 additions & 1 deletion lib/Basic/SourceLoc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,8 @@ StringRef SourceManager::extractText(CharSourceRange Range,
Range.getByteLength());
}

unsigned SourceManager::findBufferContainingLoc(SourceLoc Loc) const {
Optional<unsigned>
SourceManager::findBufferContainingLocInternal(SourceLoc Loc) const {
assert(Loc.isValid());
// Search the buffers back-to front, so later alias buffers are
// visited first.
Expand All @@ -226,9 +227,20 @@ unsigned SourceManager::findBufferContainingLoc(SourceLoc Loc) const {
less_equal(Loc.Value.getPointer(), Buf->getBufferEnd()))
return i;
}
return None;
}

unsigned SourceManager::findBufferContainingLoc(SourceLoc Loc) const {
auto Id = findBufferContainingLocInternal(Loc);
if (Id.hasValue())
return *Id;
llvm_unreachable("no buffer containing location found");
}

bool SourceManager::isOwning(SourceLoc Loc) const {
return findBufferContainingLocInternal(Loc).hasValue();
}

void SourceRange::widen(SourceRange Other) {
if (Other.Start.Value.getPointer() < Start.Value.getPointer())
Start = Other.Start;
Expand Down
21 changes: 15 additions & 6 deletions test/api-digester/compare-dump-abi.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,20 @@
// RUN: %empty-directory(%t.mod2)
// RUN: %empty-directory(%t.sdk)
// RUN: %empty-directory(%t.module-cache)
// RUN: %swift -emit-module -o %t.mod1/cake.swiftmodule %S/Inputs/cake_baseline/cake.swift -parse-as-library -enable-library-evolution -I %S/Inputs/APINotesLeft %clang-importer-sdk-nosource
// RUN: %swift -emit-module -o %t.mod2/cake.swiftmodule %S/Inputs/cake_current/cake.swift -parse-as-library -enable-library-evolution -I %S/Inputs/APINotesRight %clang-importer-sdk-nosource
// RUN: %api-digester -dump-sdk -module cake -o - -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod1 -I %S/Inputs/APINotesLeft -abi > %t.dump1.json
// RUN: %api-digester -dump-sdk -module cake -o - -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod2 -I %S/Inputs/APINotesLeft -abi > %t.dump2.json
// RUN: %api-digester -diagnose-sdk -print-module --input-paths %t.dump1.json -input-paths %t.dump2.json -abi -o %t.result
// RUN: %swift -emit-module -o %t.mod1/cake.swiftmodule %S/Inputs/cake_baseline/cake.swift -parse-as-library -enable-library-evolution -I %S/Inputs/APINotesLeft %clang-importer-sdk-nosource -emit-module-source-info -emit-module-source-info-path %t.mod1/cake.swiftsourceinfo
// RUN: %swift -emit-module -o %t.mod2/cake.swiftmodule %S/Inputs/cake_current/cake.swift -parse-as-library -enable-library-evolution -I %S/Inputs/APINotesRight %clang-importer-sdk-nosource -emit-module-source-info -emit-module-source-info-path %t.mod2/cake.swiftsourceinfo
// RUN: %api-digester -dump-sdk -module cake -o %t.dump1.json -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod1 -I %S/Inputs/APINotesLeft -abi
// RUN: %api-digester -diagnose-sdk -print-module -baseline-path %t.dump1.json -module cake -I %t.mod2 -I %S/Inputs/APINotesLeft -module-cache-path %t.module-cache %clang-importer-sdk-nosource -abi -o %t.result
// RUN: %clang -E -P -x c %S/Outputs/Cake-abi.txt -o - | sed '/^\s*$/d' > %t.abi.expected
// RUN: %clang -E -P -x c %t.result -o - | sed '/^\s*$/d' > %t.abi.result.tmp
// RUN: diff -u %t.abi.expected %t.abi.result.tmp
// RUN: diff -u %t.abi.expected %t.abi.result.tmp

// A compiler-style diag to ensure we have source locations associated with breakages.
// RUN: not %api-digester -diagnose-sdk -print-module -baseline-path %t.dump1.json -module cake -I %t.mod2 -I %S/Inputs/APINotesLeft -module-cache-path %t.module-cache %clang-importer-sdk-nosource -abi -o %t.result -compiler-style-diags 2> %t.abi.compiler.diags
// RUN: %FileCheck %s < %t.abi.compiler.diags

// CHECK: cake_current/cake.swift:31:14: error: cake: Class C4 has changed its super class from APINotesTest.OldType to APINotesTest.NewType
// CHECK: cake_current/cake.swift:33:14: error: cake: Class C5 is now without @objc
// CHECK: cake_current/cake.swift:35:23: error: cake: Func C5.dy_foo() is now with dynamic
// CHECK: cake_current/cake.swift:39:15: error: cake: Struct C6 is now with @frozen
// CHECK: cake_current/cake.swift:41:13: error: cake: Enum IceKind is now without @frozen
36 changes: 31 additions & 5 deletions tools/swift-api-digester/ModuleAnalyzerNodes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ struct swift::ide::api::SDKNodeInitInfo {
SDKContext &Ctx;
DeclKind DKind;
AccessorKind AccKind;

SourceLoc Loc;
#define KEY_STRING(X, Y) StringRef X;
#include "swift/IDE/DigesterEnums.def"
#define KEY_BOOL(X, Y) bool X = false;
Expand All @@ -52,6 +52,29 @@ struct swift::ide::api::SDKNodeInitInfo {

SDKContext::SDKContext(CheckerOptions Opts): Diags(SourceMgr), Opts(Opts) {}

DiagnosticEngine &SDKContext::getDiags(SourceLoc Loc) {
// If the location is invalid, we just use the locally created DiagEngine.
if (Loc.isInvalid())
return Diags;
// If the Loc is valid, it may belong to any of the SourceManagers owned by
// the ASTContexts we created, thus we should go through the ASTContxts to find
// the right DiagnosticEngine to use.
for (auto &CI: CIs) {
if (CI->getSourceMgr().isOwning(Loc))
return CI->getDiags();
}
llvm_unreachable("cannot find diagnostic engine to use");
}

void SDKContext::addDiagConsumer(DiagnosticConsumer &Consumer) {
// we may emit diagnostics via any of the diagnostic engine, so add the consumer
// to all of them.
Diags.addConsumer(Consumer);
for (auto &CI: CIs) {
CI->getDiags().addConsumer(Consumer);
}
}

void SDKNodeRoot::registerDescendant(SDKNode *D) {
// Operator doesn't have usr
if (isa<SDKNodeDeclOperator>(D))
Expand All @@ -70,7 +93,7 @@ SDKNodeRoot::SDKNodeRoot(SDKNodeInitInfo Info): SDKNode(Info, SDKNodeKind::Root)
JsonFormatVer(Info.JsonFormatVer.hasValue() ? *Info.JsonFormatVer : DIGESTER_JSON_DEFAULT_VERSION) {}

SDKNodeDecl::SDKNodeDecl(SDKNodeInitInfo Info, SDKNodeKind Kind)
: SDKNode(Info, Kind), DKind(Info.DKind), Usr(Info.Usr),
: SDKNode(Info, Kind), DKind(Info.DKind), Usr(Info.Usr), Loc(Info.Loc),
Location(Info.Location), ModuleName(Info.ModuleName),
DeclAttributes(Info.DeclAttrs), IsImplicit(Info.IsImplicit),
IsStatic(Info.IsStatic), IsDeprecated(Info.IsDeprecated),
Expand Down Expand Up @@ -1297,7 +1320,7 @@ StringRef SDKContext::getInitKind(Decl *D) {
}

SDKNodeInitInfo::SDKNodeInitInfo(SDKContext &Ctx, Decl *D):
Ctx(Ctx), DKind(D->getKind()),
Ctx(Ctx), DKind(D->getKind()), Loc(D->getLoc()),
Location(calculateLocation(Ctx, D)),
ModuleName(D->getModuleContext()->getName().str()),
GenericSig(printGenericSignature(Ctx, D, /*Canonical*/Ctx.checkingABI())),
Expand Down Expand Up @@ -2143,6 +2166,9 @@ swift::ide::api::getSDKNodeRoot(SDKContext &SDKCtx,
if (llvm::errs().has_colors())
PrintDiags.forceColors();
CI.addDiagnosticConsumer(&PrintDiags);
// The PrintDiags is only responsible compiler errors, we should remove the
// consumer immediately after importing is done.
SWIFT_DEFER { CI.getDiags().removeConsumer(PrintDiags); };
if (CI.setup(Invocation)) {
llvm::errs() << "Failed to setup the compiler instance\n";
return nullptr;
Expand Down Expand Up @@ -2211,7 +2237,7 @@ int swift::ide::api::deserializeSDKDump(StringRef dumpPath, StringRef OutputPath
}
PrintingDiagnosticConsumer PDC;
SDKContext Ctx(Opts);
Ctx.getDiags().addConsumer(PDC);
Ctx.addDiagConsumer(PDC);

SwiftDeclCollector Collector(Ctx);
Collector.deSerialize(dumpPath);
Expand All @@ -2227,7 +2253,7 @@ int swift::ide::api::findDeclUsr(StringRef dumpPath, CheckerOptions Opts) {
}
PrintingDiagnosticConsumer PDC;
SDKContext Ctx(Opts);
Ctx.getDiags().addConsumer(PDC);
Ctx.addDiagConsumer(PDC);

SwiftDeclCollector Collector(Ctx);
Collector.deSerialize(dumpPath);
Expand Down
14 changes: 8 additions & 6 deletions tools/swift-api-digester/ModuleAnalyzerNodes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,9 @@ class SDKContext {
SourceManager &getSourceMgr() {
return SourceMgr;
}
DiagnosticEngine &getDiags() {
return Diags;
}
// Find a DiagnosticEngine to use when emitting diagnostics at the given Loc.
DiagnosticEngine &getDiags(SourceLoc Loc = SourceLoc());
void addDiagConsumer(DiagnosticConsumer &Consumer);
void setCommonVersion(uint8_t Ver) {
assert(!CommonVersion.hasValue());
CommonVersion = Ver;
Expand Down Expand Up @@ -337,6 +337,7 @@ struct PlatformIntroVersion {
class SDKNodeDecl: public SDKNode {
DeclKind DKind;
StringRef Usr;
SourceLoc Loc;
StringRef Location;
StringRef ModuleName;
std::vector<DeclAttrKind> DeclAttributes;
Expand Down Expand Up @@ -393,20 +394,21 @@ class SDKNodeDecl: public SDKNode {
uint8_t getFixedBinaryOrder() const { return *FixedBinaryOrder; }
PlatformIntroVersion getIntroducingVersion() const { return introVersions; }
StringRef getObjCName() const { return ObjCName; }
SourceLoc getLoc() const { return Loc; }
virtual void jsonize(json::Output &Out) override;
virtual void diagnose(SDKNode *Right) override;

// The first argument of the diag is always screening info.
template<typename ...ArgTypes>
void emitDiag(Diag<StringRef, ArgTypes...> ID,
void emitDiag(SourceLoc Loc,
Diag<StringRef, ArgTypes...> ID,
typename detail::PassArgument<ArgTypes>::type... Args) const {
// Don't emit objc decls if we care about swift exclusively
if (Ctx.getOpts().SwiftOnly) {
if (isObjc())
return;
}
Ctx.getDiags().diagnose(SourceLoc(), ID, getScreenInfo(),
std::move(Args)...);
Ctx.getDiags(Loc).diagnose(Loc, ID, getScreenInfo(), std::move(Args)...);
}
};

Expand Down
Loading