Merge pull request swiftlang#33720 from nkcsgexi/add-allow-list-abi-checker

nkcsgexi · web-flow · commit e470919a4f79 · 2020-08-31T13:33:22.000-07:00
ABI checker: add a mechansim for specifying allowed ABI breakages
diff --git a/include/swift/AST/DiagnosticsModuleDiffer.def b/include/swift/AST/DiagnosticsModuleDiffer.def
@@ -74,7 +74,7 @@ ERROR(param_ownership_change,none,"%0 has %1 changing from %2 to %3", (StringRef
 
 ERROR(type_witness_change,none,"%0 has type witness type for %1 changing from %2 to %3", (StringRef, StringRef, StringRef, StringRef))
 
-ERROR(decl_new_witness_table_entry,none,"%0 now requires %select{|no}1 new witness table entry", (StringRef, bool))
+ERROR(decl_new_witness_table_entry,none,"%0 now requires%select{| no}1 new witness table entry", (StringRef, bool))
 
 ERROR(new_decl_without_intro,none,"%0 is a new API without @available attribute", (StringRef))
 
@@ -88,5 +88,7 @@ ERROR(not_inheriting_convenience_inits,none,"%0 no longer inherits convenience i
 
 ERROR(enum_case_added,none,"%0 has been added as a new enum case", (StringRef))
 
+WARNING(cannot_read_allowlist,none,"cannot read breakage allowlist at '%0'", (StringRef))
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"
diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h
@@ -378,6 +378,7 @@ class ClangImporter final : public ClangModuleLoader {
   /// construction of the replica.
   bool dumpPrecompiledModule(StringRef modulePath, StringRef outputPath);
 
+  bool runPreprocessor(StringRef inputPath, StringRef outputPath);
   const clang::Module *getClangOwningModule(ClangNode Node) const;
   bool hasTypedef(const clang::Decl *typeDecl) const;
 
diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp
@@ -1597,6 +1597,30 @@ ClangImporter::emitBridgingPCH(StringRef headerPath,
   return false;
 }
 
+bool ClangImporter::runPreprocessor(StringRef inputPath, StringRef outputPath) {
+  auto emitInstance = cloneCompilerInstanceForPrecompiling();
+  auto &invocation = emitInstance->getInvocation();
+  auto LangOpts = invocation.getLangOpts();
+  auto &OutputOpts = invocation.getPreprocessorOutputOpts();
+  OutputOpts.ShowCPP = 1;
+  OutputOpts.ShowComments = 0;
+  OutputOpts.ShowLineMarkers = 0;
+  OutputOpts.ShowMacros = 0;
+  OutputOpts.ShowMacroComments = 0;
+  auto language = getLanguageFromOptions(LangOpts);
+  auto inputFile = clang::FrontendInputFile(inputPath, language);
+
+  auto &FrontendOpts = invocation.getFrontendOpts();
+  FrontendOpts.Inputs = {inputFile};
+  FrontendOpts.OutputFile = outputPath.str();
+  FrontendOpts.ProgramAction = clang::frontend::PrintPreprocessedInput;
+
+  auto action = wrapActionForIndexingIfEnabled(
+      FrontendOpts, std::make_unique<clang::PrintPreprocessedAction>());
+  emitInstance->ExecuteAction(*action);
+  return emitInstance->getDiagnostics().hasErrorOccurred();
+}
+
 bool ClangImporter::emitPrecompiledModule(StringRef moduleMapPath,
                                           StringRef moduleName,
                                           StringRef outputPath) {
diff --git a/test/api-digester/breakage-allowlist.swift b/test/api-digester/breakage-allowlist.swift
@@ -0,0 +1,38 @@
+// REQUIRES: VENDOR=apple
+
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t.mod1)
+// RUN: %empty-directory(%t.mod2)
+// RUN: %empty-directory(%t.sdk)
+// RUN: %empty-directory(%t.module-cache)
+// RUN: %empty-directory(%t.baseline/ABI)
+
+// RUN: echo "public func foo() {}" > %t.mod1/Foo.swift
+// RUN: echo "public func bar() {}" > %t.mod2/Foo.swift
+
+// RUN: echo "Foo: Func foo() has been removed" > %t/incomplete-allowlist.txt
+// RUN: echo "Foo: Func foo() has been removed" > %t/complete-allowlist.txt
+// RUN: echo "Foo: Func bar() is a new API without @available attribute" >> %t/complete-allowlist.txt
+
+// RUN: %target-swift-frontend -disable-objc-attr-requires-foundation-module -emit-module -o %t.mod1/Foo.swiftmodule  %t.mod1/Foo.swift -parse-as-library -enable-library-evolution -emit-module-source-info -emit-module-source-info-path %t.mod1/Foo.swiftsourceinfo -emit-module-interface-path %t.mod1/Foo.swiftinterface -module-name Foo -swift-version 5
+
+// RUN: %target-swift-frontend -disable-objc-attr-requires-foundation-module -emit-module -o %t.mod2/Foo.swiftmodule  %t.mod2/Foo.swift -parse-as-library -enable-library-evolution -emit-module-source-info -emit-module-source-info-path %t.mod2/Foo.swiftsourceinfo -emit-module-interface-path %t.mod2/Foo.swiftinterface -module-name Foo -swift-version 5
+
+// RUN: %api-digester -dump-sdk -module Foo -output-dir %t.baseline -module-cache-path %t.module-cache %clang-importer-sdk-nosource -I %t.mod1 -abi -use-interface-for-module Foo
+
+// RUN: %api-digester -diagnose-sdk -print-module -baseline-dir %t.baseline -module Foo -I %t.mod2 -module-cache-path %t.module-cache %clang-importer-sdk-nosource -abi -breakage-allowlist-path %t/complete-allowlist.txt -o %t/expected-diags.txt
+
+// RUN: not %api-digester -diagnose-sdk -print-module -baseline-dir %t.baseline -module Foo -I %t.mod2 -module-cache-path %t.module-cache %clang-importer-sdk-nosource -abi -compiler-style-diags
+
+// RUN: not %api-digester -diagnose-sdk -print-module -baseline-dir %t.baseline -module Foo -I %t.mod2 -module-cache-path %t.module-cache %clang-importer-sdk-nosource -abi -compiler-style-diags -breakage-allowlist-path %t/incomplete-allowlist.txt
+
+// RUN: %api-digester -diagnose-sdk -print-module -baseline-dir %t.baseline -module Foo -I %t.mod2 -module-cache-path %t.module-cache %clang-importer-sdk-nosource -abi -compiler-style-diags -breakage-allowlist-path %t/complete-allowlist.txt
+
+// RUN: not %api-digester -diagnose-sdk -print-module -baseline-dir %t.baseline -module Foo -I %t.mod2 -module-cache-path %t.module-cache %clang-importer-sdk-nosource -abi -serialize-diagnostics-path %t/serialized-diag.dia
+// RUN: ls %t/serialized-diag.dia
+
+// RUN: not %api-digester -diagnose-sdk -print-module -baseline-dir %t.baseline -module Foo -I %t.mod2 -module-cache-path %t.module-cache %clang-importer-sdk-nosource -abi -serialize-diagnostics-path %t/serialized-diag.dia -breakage-allowlist-path %t/incomplete-allowlist.txt
+// RUN: ls %t/serialized-diag.dia
+
+// RUN: %api-digester -diagnose-sdk -print-module -baseline-dir %t.baseline -module Foo -I %t.mod2 -module-cache-path %t.module-cache %clang-importer-sdk-nosource -abi -serialize-diagnostics-path %t/serialized-diag.dia -breakage-allowlist-path %t/complete-allowlist.txt
+// RUN: ls %t/serialized-diag.dia
diff --git a/tools/swift-api-digester/ModuleDiagsConsumer.cpp b/tools/swift-api-digester/ModuleDiagsConsumer.cpp
@@ -78,7 +78,7 @@ static StringRef getCategoryName(uint32_t ID) {
   case LocalDiagID::not_inheriting_convenience_inits:
     return "/* Class Inheritance Change */";
   default:
-    return StringRef();
+    return "/* Others */";
   }
 }
 }
@@ -122,3 +122,28 @@ swift::ide::api::ModuleDifferDiagsConsumer::~ModuleDifferDiagsConsumer() {
     }
   }
 }
+
+bool swift::ide::api::
+FilteringDiagnosticConsumer::shouldProceed(const DiagnosticInfo &Info) {
+  if (allowedBreakages->empty()) {
+    return true;
+  }
+  llvm::SmallString<256> Text;
+  {
+    llvm::raw_svector_ostream Out(Text);
+    DiagnosticEngine::formatDiagnosticText(Out, Info.FormatString,
+                                           Info.FormatArgs);
+  }
+  return allowedBreakages->count(Text.str()) == 0;
+}
+
+void swift::ide::api::
+FilteringDiagnosticConsumer::handleDiagnostic(SourceManager &SM,
+                                              const DiagnosticInfo &Info) {
+  if (shouldProceed(Info)) {
+    if (Info.Kind == DiagnosticKind::Error) {
+      HasError = true;
+    }
+    subConsumer->handleDiagnostic(SM, Info);
+  }
+}
diff --git a/tools/swift-api-digester/ModuleDiagsConsumer.h b/tools/swift-api-digester/ModuleDiagsConsumer.h
@@ -41,6 +41,30 @@ class ModuleDifferDiagsConsumer: public PrintingDiagnosticConsumer {
   ~ModuleDifferDiagsConsumer();
   void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;
 };
+
+class FilteringDiagnosticConsumer: public DiagnosticConsumer {
+  bool HasError = false;
+  std::unique_ptr<DiagnosticConsumer> subConsumer;
+  std::unique_ptr<llvm::StringSet<>> allowedBreakages;
+  bool shouldProceed(const DiagnosticInfo &Info);
+public:
+  FilteringDiagnosticConsumer(std::unique_ptr<DiagnosticConsumer> subConsumer,
+                              std::unique_ptr<llvm::StringSet<>> allowedBreakages):
+    subConsumer(std::move(subConsumer)),
+    allowedBreakages(std::move(allowedBreakages)) {}
+  ~FilteringDiagnosticConsumer() = default;
+
+  bool finishProcessing() override { return subConsumer->finishProcessing(); }
+  bool hasError() const { return HasError; }
+  void flush() override { subConsumer->flush(); }
+
+  void informDriverOfIncompleteBatchModeCompilation() override {
+    subConsumer->informDriverOfIncompleteBatchModeCompilation();
+  }
+
+  void handleDiagnostic(SourceManager &SM,
+                        const DiagnosticInfo &Info) override;
+};
 }
 }
 }
diff --git a/tools/swift-api-digester/swift-api-digester.cpp b/tools/swift-api-digester/swift-api-digester.cpp
@@ -264,6 +264,11 @@ SerializedDiagPath("serialize-diagnostics-path",
                    llvm::cl::desc("Serialize diagnostics to a path"),
                    llvm::cl::cat(Category));
 
+static llvm::cl::opt<std::string>
+BreakageAllowlistPath("breakage-allowlist-path",
+                      llvm::cl::desc("An allowlist of breakages to not complain about"),
+                      llvm::cl::cat(Category));
+
 } // namespace options
 
 namespace {
@@ -2320,6 +2325,50 @@ createDiagConsumer(llvm::raw_ostream &OS, bool &FailOnError) {
   }
 }
 
+static int readFileLineByLine(StringRef Path, llvm::StringSet<> &Lines) {
+  auto FileBufOrErr = llvm::MemoryBuffer::getFile(Path);
+  if (!FileBufOrErr) {
+    llvm::errs() << "error opening file '" << Path << "': "
+      << FileBufOrErr.getError().message() << '\n';
+    return 1;
+  }
+
+  StringRef BufferText = FileBufOrErr.get()->getBuffer();
+  while (!BufferText.empty()) {
+    StringRef Line;
+    std::tie(Line, BufferText) = BufferText.split('\n');
+    Line = Line.trim();
+    if (Line.empty())
+      continue;
+    if (Line.startswith("// ")) // comment.
+      continue;
+    Lines.insert(Line);
+  }
+  return 0;
+}
+
+static bool readBreakageAllowlist(SDKContext &Ctx, llvm::StringSet<> &lines) {
+  if (options::BreakageAllowlistPath.empty())
+    return 0;
+  CompilerInstance instance;
+  CompilerInvocation invok;
+  invok.setModuleName("ForClangImporter");
+  if (instance.setup(invok))
+    return 1;
+  ClangImporterOptions impOpts;
+  auto importer = ClangImporter::create(instance.getASTContext(), impOpts);
+  SmallString<128> preprocessedFilePath;
+  if (auto error = llvm::sys::fs::createTemporaryFile(
+    "breakage-allowlist-", "txt", preprocessedFilePath)) {
+    return 1;
+  }
+  if (importer->runPreprocessor(options::BreakageAllowlistPath,
+                                preprocessedFilePath.str())) {
+    return 1;
+  }
+  return readFileLineByLine(preprocessedFilePath, lines);
+}
+
 static int diagnoseModuleChange(SDKContext &Ctx, SDKNodeRoot *LeftModule,
                              SDKNodeRoot *RightModule, StringRef OutputPath,
                              llvm::StringSet<> ProtocolReqAllowlist) {
@@ -2337,9 +2386,14 @@ static int diagnoseModuleChange(SDKContext &Ctx, SDKNodeRoot *LeftModule,
     OS = FileOS.get();
   }
   bool FailOnError;
-  std::unique_ptr<DiagnosticConsumer> pConsumer =
-    createDiagConsumer(*OS, FailOnError);
-
+  auto allowedBreakages = std::make_unique<llvm::StringSet<>>();
+  if (readBreakageAllowlist(Ctx, *allowedBreakages)) {
+    Ctx.getDiags().diagnose(SourceLoc(), diag::cannot_read_allowlist,
+                            options::BreakageAllowlistPath);
+  }
+  auto pConsumer = std::make_unique<FilteringDiagnosticConsumer>(
+    createDiagConsumer(*OS, FailOnError), std::move(allowedBreakages));
+  SWIFT_DEFER { pConsumer->finishProcessing(); };
   Ctx.addDiagConsumer(*pConsumer);
   Ctx.setCommonVersion(std::min(LeftModule->getJsonFormatVersion(),
                                 RightModule->getJsonFormatVersion()));
@@ -2352,7 +2406,7 @@ static int diagnoseModuleChange(SDKContext &Ctx, SDKNodeRoot *LeftModule,
   // Find member hoist changes to help refine diagnostics.
   findTypeMemberDiffs(LeftModule, RightModule, Ctx.getTypeMemberDiffs());
   DiagnosisEmitter::diagnosis(LeftModule, RightModule, Ctx);
-  return FailOnError && Ctx.getDiags().hadAnyError() ? 1 : 0;
+  return FailOnError && pConsumer->hasError() ? 1 : 0;
 }
 
 static int diagnoseModuleChange(StringRef LeftPath, StringRef RightPath,
@@ -2493,28 +2547,6 @@ static int generateMigrationScript(StringRef LeftPath, StringRef RightPath,
   return 0;
 }
 
-static int readFileLineByLine(StringRef Path, llvm::StringSet<> &Lines) {
-  auto FileBufOrErr = llvm::MemoryBuffer::getFile(Path);
-  if (!FileBufOrErr) {
-    llvm::errs() << "error opening file '" << Path << "': "
-      << FileBufOrErr.getError().message() << '\n';
-    return 1;
-  }
-
-  StringRef BufferText = FileBufOrErr.get()->getBuffer();
-  while (!BufferText.empty()) {
-    StringRef Line;
-    std::tie(Line, BufferText) = BufferText.split('\n');
-    Line = Line.trim();
-    if (Line.empty())
-      continue;
-    if (Line.startswith("// ")) // comment.
-      continue;
-    Lines.insert(Line);
-  }
-  return 0;
-}
-
 // This function isn't referenced outside its translation unit, but it
 // can't use the "static" keyword because its address is used for
 // getMainExecutable (since some platforms don't support taking the
