Skip to content

Compilation cleanup #7827

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.

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 21 commits into from
Mar 2, 2017
Merged

Compilation cleanup #7827

merged 21 commits into from
Mar 2, 2017

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Feb 28, 2017

Some generic refactoring and tidying in the driver, mostly splitting Compilation::performJobsImpl
into enough pieces (and turning various lambdas and locals into named and commented methods
and members) that I can follow what it's doing. This is prep work for more involved debugging of
driver issues; I realized I couldn't really understand the structure of performJobsImpl before.

Along the way, fixed a few minor, mostly-logging issues:

  • Jobs are all now logged uniformly
  • Link and compile jobs look different now (this accounted for seemingly redundant log messages)
  • Skipped jobs are only logged and marked finished once, not once-per-taskqueue-iteration
  • MarkTracer is provided to all markTransitive calls, so it logs dependency-reasons more often
  • The confusing "" member provision case is logged as a special case
  • Failure to reload swiftdeps files when expected will emit a warning before rebuilding the world

rdar://30792302

@graydon graydon requested a review from jrose-apple February 28, 2017 23:26
@@ -266,6 +266,18 @@ namespace driver {
parseable_output::emitBeganMessage(llvm::errs(), *BeganCmd, Pid);
}

void
dependencyLoadFailed(StringRef DependenciesFile, bool Warn=true) {
if (Warn)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on the fence about this. On the one hand, we might get more feedback from users, but on the other they may end up with a warning they can't silence. (Neither Xcode nor SwiftPM expose a switch to turn off incremental builds.)

}
DeferredCommands.clear();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, good catch.

@jrose-apple
Copy link
Contributor

I'm going to trust you that all the refactoring parts don't change anything; chopping up performJobsImpl is certainly not a bad thing. I think "PerformJobsState" may no longer be the right name for the thing you've created.

@graydon
Copy link
Contributor Author

graydon commented Mar 1, 2017

Pushed a couple updates to dial back the diagnostic chatter.

@graydon graydon force-pushed the compilation-cleanup branch from 50ebd69 to 5c19107 Compare March 1, 2017 02:06
@graydon graydon force-pushed the compilation-cleanup branch from 5c19107 to 1c3a54b Compare March 1, 2017 17:07
@graydon
Copy link
Contributor Author

graydon commented Mar 1, 2017

@swift-ci please test and merge

@graydon
Copy link
Contributor Author

graydon commented Mar 1, 2017

@swift-ci please smoke test and merge

@graydon
Copy link
Contributor Author

graydon commented Mar 1, 2017

@swift-ci please smoke test

@graydon graydon merged commit babf6ff into swiftlang:master Mar 2, 2017
@graydon graydon deleted the compilation-cleanup branch September 13, 2017 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants