Merge pull request swiftlang#33756 from nkcsgexi/module-loader-opts

nkcsgexi · web-flow · commit ed5edb89f58e · 2020-09-02T18:20:35.000-07:00
ModuleLoader: remove several walk-arounds for not having a persistent ClangImporterOptions available. NFC
diff --git a/include/swift/ClangImporter/ClangImporter.h b/include/swift/ClangImporter/ClangImporter.h
@@ -472,7 +472,6 @@ class ClangImporter final : public ClangModuleLoader {
 
   bool isSerializable(const clang::Type *type,
                       bool checkCanonical) const override;
-  ArrayRef<std::string> getExtraClangArgs() const;
 };
 
 ImportDecl *createImportDecl(ASTContext &Ctx, DeclContext *DC, ClangNode ClangN,
diff --git a/include/swift/Frontend/ModuleInterfaceLoader.h b/include/swift/Frontend/ModuleInterfaceLoader.h
@@ -420,8 +420,8 @@ struct InterfaceSubContextDelegateImpl: InterfaceSubContextDelegate {
                                   DiagnosticEngine &Diags,
                                   const SearchPathOptions &searchPathOpts,
                                   const LangOptions &langOpts,
+                                  const ClangImporterOptions &clangImporterOpts,
                                   ModuleInterfaceLoaderOptions LoaderOpts,
-                                  ClangModuleLoader *clangImporter,
                                   bool buildModuleCacheDirIfAbsent,
                                   StringRef moduleCachePath,
                                   StringRef prebuiltCachePath,
@@ -448,7 +448,6 @@ struct InterfaceSubContextDelegateImpl: InterfaceSubContextDelegate {
                                     llvm::SmallString<256> &OutPath,
                                     StringRef &CacheHash);
   std::string getCacheHash(StringRef useInterfacePath);
-  void addExtraClangArg(StringRef Arg);
 };
 }
 
diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp
@@ -969,10 +969,6 @@ ClangImporter::createClangInvocation(ClangImporter *importer,
                                                 nullptr, false, CC1Args);
 }
 
-ArrayRef<std::string> ClangImporter::getExtraClangArgs() const {
-  return Impl.ExtraClangArgs;
-}
-
 std::unique_ptr<ClangImporter>
 ClangImporter::create(ASTContext &ctx,
                       std::string swiftPCHHash, DependencyTracker *tracker,
@@ -982,7 +978,6 @@ ClangImporter::create(ASTContext &ctx,
   auto &importerOpts = ctx.ClangImporterOpts;
   importer->Impl.ClangArgs = getClangArguments(ctx);
   ArrayRef<std::string> invocationArgStrs = importer->Impl.ClangArgs;
-  importer->Impl.ExtraClangArgs = importerOpts.ExtraArgs;
   if (importerOpts.DumpClangDiagnostics) {
     llvm::errs() << "'";
     llvm::interleave(
diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h
@@ -393,8 +393,6 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   /// Clang arguments used to create the Clang invocation.
   std::vector<std::string> ClangArgs;
 
-  /// Extra clang args specified via "-Xcc"
-  std::vector<std::string> ExtraClangArgs;
 public:
   /// Mapping of already-imported declarations.
   llvm::DenseMap<std::pair<const clang::Decl *, Version>, Decl *> ImportedDecls;
diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp
@@ -538,8 +538,8 @@ bool CompilerInstance::setUpModuleLoaders() {
     ModuleInterfaceLoaderOptions LoaderOpts(FEOpts);
     InterfaceSubContextDelegateImpl ASTDelegate(Context->SourceMgr, Context->Diags,
                                                 Context->SearchPathOpts, Context->LangOpts,
+                                                Context->ClangImporterOpts,
                                                 LoaderOpts,
-                                                Context->getClangModuleLoader(),
                                                 /*buildModuleCacheDirIfAbsent*/false,
                                                 ModuleCachePath,
                                                 FEOpts.PrebuiltModuleCachePath,
diff --git a/lib/Frontend/ModuleInterfaceLoader.cpp b/lib/Frontend/ModuleInterfaceLoader.cpp
@@ -844,8 +844,8 @@ class ModuleInterfaceLoaderImpl {
     }
     InterfaceSubContextDelegateImpl astDelegate(ctx.SourceMgr, ctx.Diags,
                                                 ctx.SearchPathOpts, ctx.LangOpts,
+                                                ctx.ClangImporterOpts,
                                                 Opts,
-                                                ctx.getClangModuleLoader(),
                                                 /*buildModuleCacheDirIfAbsent*/true,
                                                 cacheDir,
                                                 prebuiltCacheDir,
@@ -1084,20 +1084,12 @@ bool ModuleInterfaceLoader::buildSwiftModuleFromSwiftInterface(
     bool SerializeDependencyHashes, bool TrackSystemDependencies,
     ModuleInterfaceLoaderOptions LoaderOpts) {
   InterfaceSubContextDelegateImpl astDelegate(SourceMgr, Diags,
-                                              SearchPathOpts, LangOpts,
+                                              SearchPathOpts, LangOpts, ClangOpts,
                                               LoaderOpts,
-                                              /*clangImporter*/nullptr,
                                               /*CreateCacheDirIfAbsent*/true,
                                               CacheDir, PrebuiltCacheDir,
                                               SerializeDependencyHashes,
                                               TrackSystemDependencies);
-  // At this point we don't have an ClangImporter instance because the instance
-  // is created later when we create a new ASTContext to build the interface.
-  // Thus, we have to add these extra clang flags manually here to ensure explict
-  // module building works.
-  for (auto &Arg: ClangOpts.ExtraArgs) {
-    astDelegate.addExtraClangArg(Arg);
-  }
   ModuleInterfaceBuilder builder(SourceMgr, Diags, astDelegate, InPath,
                                  ModuleName, CacheDir, PrebuiltCacheDir,
                                  LoaderOpts.disableInterfaceLock);
@@ -1238,19 +1230,13 @@ bool InterfaceSubContextDelegateImpl::extractSwiftInterfaceVersionAndArgs(
   return false;
 }
 
-void InterfaceSubContextDelegateImpl::addExtraClangArg(StringRef arg) {
-  genericSubInvocation.getClangImporterOptions().ExtraArgs.push_back(arg);
-  GenericArgs.push_back("-Xcc");
-  GenericArgs.push_back(ArgSaver.save(arg));
-}
-
 InterfaceSubContextDelegateImpl::InterfaceSubContextDelegateImpl(
     SourceManager &SM,
     DiagnosticEngine &Diags,
     const SearchPathOptions &searchPathOpts,
     const LangOptions &langOpts,
+    const ClangImporterOptions &clangImporterOpts,
     ModuleInterfaceLoaderOptions LoaderOpts,
-    ClangModuleLoader *clangImporter,
     bool buildModuleCacheDirIfAbsent,
     StringRef moduleCachePath,
     StringRef prebuiltCachePath,
@@ -1288,22 +1274,21 @@ InterfaceSubContextDelegateImpl::InterfaceSubContextDelegateImpl(
   StringRef explictSwiftModuleMap = searchPathOpts.ExplicitSwiftModuleMap;
   genericSubInvocation.getSearchPathOptions().ExplicitSwiftModuleMap =
     explictSwiftModuleMap;
-  if (clangImporter) {
-    // We need to add these extra clang flags because explict module building
-    // related flags are all there: -fno-implicit-modules, -fmodule-map-file=,
-    // and -fmodule-file=.
-    // If we don't add these flags, the interface will be built with implicit
-    // PCMs.
-    for (auto arg: static_cast<ClangImporter*>(clangImporter)->getExtraClangArgs()) {
-      addExtraClangArg(arg);
-    }
-    // Respect the detailed-record preprocessor setting of the parent context.
-    // This, and the "raw" clang module format it implicitly enables, are
-    // required by sourcekitd.
-    auto &Opts = clangImporter->getClangInstance().getPreprocessorOpts();
-    if (Opts.DetailedRecord) {
-      genericSubInvocation.getClangImporterOptions().DetailedPreprocessingRecord = true;
-    }
+  auto &subClangImporterOpts = genericSubInvocation.getClangImporterOptions();
+  // Respect the detailed-record preprocessor setting of the parent context.
+  // This, and the "raw" clang module format it implicitly enables, are
+  // required by sourcekitd.
+  subClangImporterOpts.DetailedPreprocessingRecord =
+    clangImporterOpts.DetailedPreprocessingRecord;
+  // We need to add these extra clang flags because explict module building
+  // related flags are all there: -fno-implicit-modules, -fmodule-map-file=,
+  // and -fmodule-file=.
+  // If we don't add these flags, the interface will be built with implicit
+  // PCMs.
+  subClangImporterOpts.ExtraArgs = clangImporterOpts.ExtraArgs;
+  for (auto arg: subClangImporterOpts.ExtraArgs) {
+    GenericArgs.push_back("-Xcc");
+    GenericArgs.push_back(ArgSaver.save(arg));
   }
 
   // Tell the genericSubInvocation to serialize dependency hashes if asked to do so.
diff --git a/lib/FrontendTool/ScanDependencies.cpp b/lib/FrontendTool/ScanDependencies.cpp
@@ -641,8 +641,8 @@ static bool scanModuleDependencies(CompilerInstance &instance,
   ModuleDependenciesCache cache;
   InterfaceSubContextDelegateImpl ASTDelegate(ctx.SourceMgr, ctx.Diags,
                                               ctx.SearchPathOpts, ctx.LangOpts,
+                                              ctx.ClangImporterOpts,
                                               LoaderOpts,
-                                              ctx.getClangModuleLoader(),
                                               /*buildModuleCacheDirIfAbsent*/false,
                                               ModuleCachePath,
                                               FEOpts.PrebuiltModuleCachePath,
@@ -839,8 +839,8 @@ bool swift::scanDependencies(CompilerInstance &instance) {
   ModuleInterfaceLoaderOptions LoaderOpts(FEOpts);
   InterfaceSubContextDelegateImpl ASTDelegate(ctx.SourceMgr, ctx.Diags,
                                               ctx.SearchPathOpts, ctx.LangOpts,
+                                              ctx.ClangImporterOpts,
                                               LoaderOpts,
-                                              ctx.getClangModuleLoader(),
                                               /*buildModuleCacheDirIfAbsent*/false,
                                               ModuleCachePath,
                                               FEOpts.PrebuiltModuleCachePath,
diff --git a/tools/swift-api-digester/swift-api-digester.cpp b/tools/swift-api-digester/swift-api-digester.cpp
@@ -2425,9 +2425,8 @@ static int diagnoseModuleChange(StringRef LeftPath, StringRef RightPath,
   LeftCollector.deSerialize(LeftPath);
   SwiftDeclCollector RightCollector(Ctx);
   RightCollector.deSerialize(RightPath);
-  diagnoseModuleChange(Ctx, LeftCollector.getSDKRoot(), RightCollector.getSDKRoot(),
-                       OutputPath, std::move(ProtocolReqAllowlist));
-  return options::CompilerStyleDiags && Ctx.getDiags().hadAnyError() ? 1 : 0;
+  return diagnoseModuleChange(Ctx, LeftCollector.getSDKRoot(),
+    RightCollector.getSDKRoot(), OutputPath, std::move(ProtocolReqAllowlist));
 }
 
 static void populateAliasChanges(NodeMap &AliasMap, DiffVector &AllItems,
