Merge pull request swiftlang#27766 from nkcsgexi/use-serialized-loc-abi-checker

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

Author: nkcsgexi

include/swift/Basic/SourceManager.h

@@ -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())
@@ -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);
 

lib/Basic/SourceLoc.cpp

@@ -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.
@@ -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;

test/api-digester/compare-dump-abi.swift

@@ -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

tools/swift-api-digester/ModuleAnalyzerNodes.cpp

@@ -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;
@@ -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))
@@ -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),
@@ -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())),
@@ -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;
@@ -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);
@@ -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);

tools/swift-api-digester/ModuleAnalyzerNodes.h

@@ -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;
@@ -337,6 +337,7 @@ struct PlatformIntroVersion {
 class SDKNodeDecl: public SDKNode {
   DeclKind DKind;
   StringRef Usr;
+  SourceLoc Loc;
   StringRef Location;
   StringRef ModuleName;
   std::vector<DeclAttrKind> DeclAttributes;
@@ -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)...);
   }
 };