Preparing CompilerInstance for batch mode. #13982

davidungar · 2018-01-17T00:52:37Z

Use a SetVector for CompilerInstance::PrimaryBufferIDs, and create functions to test inclusion, test for WMO, and add a primary.
Split up performCompile and Invoke part 2, performCompileStepsPostSILGen, once for each primary.
Move diagnostic for emit_reference_dependencies_without_primary_file.
Includes Graydon's commits.

…SetVectors.

davidungar · 2018-01-17T00:53:13Z

@swift-ci please smoke test os x platform

davidungar · 2018-01-17T02:43:18Z

@swift-ci please smoke test linux platform

davidungar · 2018-01-17T04:45:16Z

@swift-ci please smoke test linux platform

davidungar · 2018-01-17T05:35:33Z

@swift-ci please clean test linux platform

jrose-apple

This is in pretty good shape! Great work, both of you.

jrose-apple · 2018-01-17T17:48:05Z

include/swift/Frontend/Frontend.h

+  /// Identifies the set of SourceFiles that are considered primaries. An
+  /// invariant is that any SourceFile in this set with an associated
+  /// buffer will also have its buffer ID in PrimaryBufferIDs.
+  llvm::SetVector<SourceFile*> PrimarySourceFiles;


It looks like we never test membership in this one, so you might as well just use a SmallVector (or std::vector).

jrose-apple · 2018-01-17T17:48:31Z

include/swift/Frontend/Frontend.h

+    return PrimaryBufferIDs.count(BufID) != 0;
+  }
+
+  /// Record in PrimaryBufferIDs the fact that \BufID is a primary.


Doxygen syntax: \p BufID

Fixed in two places.

There's one on this line and one above as well (at least).

Sorry--missed those. Fixed.

jrose-apple · 2018-01-17T17:49:37Z

include/swift/Frontend/Frontend.h

@@ -413,9 +431,26 @@ class CompilerInstance {
    return Invocation.getFrontendOptions().EnableSourceImport;
  }

+  /// Gets the set of SourceFiles which are the primary inputs for this
+  /// CompilerInstance.
+  const llvm::SetVector<SourceFile*> &getPrimarySourceFiles() {


…and then this can just be an ArrayRef, if we don't care about its set-ness anymore.

jrose-apple · 2018-01-17T17:51:42Z

lib/FrontendTool/FrontendTool.cpp

-    Identifier PD = PrimarySourceFile->getPrivateDiscriminator();
+  if (IRGenOpts.DebugInfoKind != IRGenDebugInfoKind::None &&
+      Instance.getPrimarySourceFile()) {
+    Identifier PD = Instance.getPrimarySourceFile()->getPrivateDiscriminator();


Hm, we're going to need to do something better here soon.

jrose-apple · 2018-01-17T17:53:08Z

include/swift/Frontend/Frontend.h

@@ -437,6 +437,16 @@ class CompilerInstance {
    return PrimarySourceFiles;
  }

+  /// Gets the Primary Source File if one exists, otherwise the main


A list of conflicts crept into the commit message here. ("Sink a bunch of FrontendTool uses…")

Removed them.

jrose-apple · 2018-01-17T17:53:56Z

lib/FrontendTool/FrontendTool.cpp

-  auto astGuaranteedToCorrespondToSIL = false;
-  if (!SM) {
+  // The second boolean in each std::pair<> in this std::deque<> indicates
+  // whether the SIL is guaranteed to correspond to the the AST. This might be


Typo: "the the"

No longer applies, AFAICT.

jrose-apple · 2018-01-17T17:56:05Z

lib/FrontendTool/FrontendTool.cpp

@@ -504,10 +504,17 @@ createOptRecordFile(StringRef Filename, DiagnosticEngine &DE) {
  return File;
 }

+struct PostSILGenInputs {


"Conflicts" in this commit message too ("Thread the ModuleOrSourceFile of each…")

Removed the conflicts parts of the commit messages.

jrose-apple · 2018-01-17T17:56:40Z

lib/FrontendTool/FrontendTool.cpp

@@ -504,10 +504,17 @@ createOptRecordFile(StringRef Filename, DiagnosticEngine &DE) {
  return File;
 }

+struct PostSILGenInputs {
+  std::unique_ptr<SILModule> TheSILModule;
+  bool astGuaranteedToCorrespondToSIL;


Please be consistent about initial case across struct fields.

Changed to AST...

jrose-apple · 2018-01-17T17:57:20Z

lib/FrontendTool/FrontendTool.cpp

  }

-  if (SMs.empty()) {
-    auto mod = Instance.getMainModule();
+  if (PSGIs.empty()) {


Sure! Changed.

jrose-apple · 2018-01-17T17:58:08Z

lib/FrontendTool/FrontendTool.cpp

@@ -819,25 +824,26 @@ static bool performCompile(CompilerInstance &Instance,
        // once for each such input.
        for (auto *PrimaryFile : Instance.getPrimarySourceFiles()) {
          auto SM = performSILGeneration(*PrimaryFile, SILOpts, None);
-          SMs.push_back(std::make_pair(std::move(SM), !fileIsSIB(PrimaryFile)));
+          PSGIs.push_back(PostSILGenInputs{
+              std::move(SM), !fileIsSIB(PrimaryFile), PrimaryFile});


The fileIsSIB check is definitely going to be false because SIB files are a kind of SerializedASTFile, not a SourceFile.

Good catch. Replace !fileIsSIB(...) with "true".

jrose-apple · 2018-01-17T18:01:48Z

Hm, GitHub hid a bunch of the comments since you reformatting stuff afterwards, but I think they're still relevant.

…ary-input.

…ough.

…ired after changes to introduce multiple primary files to CompilerInstance. Revealed by test Frontend/dependencies.swift.

davidungar · 2018-01-17T20:01:13Z

@swift-ci please smoke test

davidungar · 2018-01-17T20:22:31Z

@swift-ci please smoke test

davidungar · 2018-01-17T20:34:11Z

@swift-ci please smoke test

jrose-apple

A few more, but they're all tiny. I think the DebugInfo one I made has already been addressed, too.

jrose-apple · 2018-01-17T21:21:42Z

include/swift/Frontend/Frontend.h

@@ -433,7 +433,7 @@ class CompilerInstance {

  /// Gets the set of SourceFiles which are the primary inputs for this
  /// CompilerInstance.
-  const llvm::SetVector<SourceFile*> &getPrimarySourceFiles() {
+  const ArrayRef<SourceFile *> getPrimarySourceFiles() {


…no need for const here again.

jrose-apple · 2018-01-17T21:22:15Z

include/swift/Frontend/Frontend.h

+    return PrimaryBufferIDs.count(BufID) != 0;
+  }
+
+  /// Record in PrimaryBufferIDs the fact that \BufID is a primary.


There's one on this line and one above as well (at least).

jrose-apple · 2018-01-17T21:22:46Z

lib/FrontendTool/FrontendTool.cpp

@@ -798,8 +798,7 @@ static bool performCompile(CompilerInstance &Instance,
  if (auto SM = Instance.takeSILModule()) {
    PSGIs.push_back(PostSILGenInputs{std::move(SM), false, mod});
  }
-
-  if (PSGIs.empty()) {
+  else if (PSGIs.empty()) {


This is just else. The deque has just been declared.

davidungar · 2018-01-17T21:30:43Z

Sorry I missed a couple of Doxygen blunders. Fixed them.

davidungar · 2018-01-17T21:47:16Z

@swift-ci please smoke test

davidungar · 2018-01-17T21:53:28Z

@swift-ci please smoke test

jrose-apple · 2018-01-17T22:10:41Z

include/swift/Frontend/Frontend.h

@@ -413,9 +431,36 @@ class CompilerInstance {
    return Invocation.getFrontendOptions().EnableSourceImport;
  }

+  /// Gets the set of SourceFiles which are the primary inputs for this
+  /// CompilerInstance.
+  const ArrayRef<SourceFile *> getPrimarySourceFiles() {


No const here.

davidungar · 2018-01-17T22:24:02Z

@swift-ci please smoke test

davidungar · 2018-01-18T00:26:17Z

@swift-ci please smoke test linux platform

[BatchMode] Change CompilerInstance::Primary{BufferID,SourceFile} to …

b0e5726

…SetVectors.

davidungar requested a review from jrose-apple January 17, 2018 03:13

jrose-apple reviewed Jan 17, 2018

View reviewed changes

graydon and others added 4 commits January 17, 2018 11:57

[BatchMode] Sink a bunch of FrontendTool uses of getPrimarySourceFile()

2494b14

[BatchMode] Split performCompile at SILGen, repeat remainder per-prim…

03c0791

…ary-input.

[BatchMode] Thread the ModuleOrSourceFile of each Post-SILGen run thr…

42c4a4d

…ough.

Fix for emit_reference_dependencies_without_primary_file warning requ…

4a0a7f2

…ired after changes to introduce multiple primary files to CompilerInstance. Revealed by test Frontend/dependencies.swift.

davidungar force-pushed the PR-18-3 branch from 824122b to 4a0a7f2 Compare January 17, 2018 19:58

David Ungar added 3 commits January 17, 2018 12:20

Changed PrimarySourceFiles from a SetVector to an ArrayRef.

1f8f994

Use \p for parameters in Doxygen comments.

9528866

Capitalized astGuaranteedToCorrespondToSIL.

6196ad7

davidungar force-pushed the PR-18-3 branch from 33228ba to 6196ad7 Compare January 17, 2018 20:22

David Ungar added 2 commits January 17, 2018 12:27

Added else to: if (PSGIs.empty())

3c882f2

Replaced !fileIsSIB test with true where it is applied to source files.

b55ec29

jrose-apple reviewed Jan 17, 2018

View reviewed changes

Fixed two more \p's.

cbd1d48

Remove redundant test: PSGIs.empty()

2d7607f

jrose-apple reviewed Jan 17, 2018

View reviewed changes

Remove useless const for getPrimarySourceFiles.

df02341

jrose-apple approved these changes Jan 18, 2018

View reviewed changes

davidungar merged commit 82d4386 into swiftlang:master Jan 18, 2018

Preparing CompilerInstance for batch mode. #13982

Preparing CompilerInstance for batch mode. #13982

Uh oh!

Conversation

davidungar commented Jan 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidungar commented Jan 17, 2018

Uh oh!

davidungar commented Jan 17, 2018

Uh oh!

davidungar commented Jan 17, 2018

Uh oh!

davidungar commented Jan 17, 2018

Uh oh!

jrose-apple left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrose-apple commented Jan 17, 2018

Uh oh!

davidungar commented Jan 17, 2018

Uh oh!

davidungar commented Jan 17, 2018

Uh oh!

davidungar commented Jan 17, 2018

Uh oh!

jrose-apple left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidungar commented Jan 17, 2018

davidungar commented Jan 17, 2018 •

edited

Loading