Skip to content

[swift-4.2-branch-06-11-2018] emit sigint for cancelled batch constituents #17233

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions include/swift/AST/DiagnosticConsumer.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,16 @@ class DiagnosticConsumer {
const DiagnosticInfo &Info) = 0;

/// \returns true if an error occurred while finishing-up.
virtual bool finishProcessing(SourceManager &) { return false; }
virtual bool finishProcessing() { return false; }

/// In batch mode, any error causes failure for all primary files, but
/// anyone consulting .dia files will only see an error for a particular
/// primary in that primary's serialized diagnostics file. For other
/// primaries' serialized diagnostics files, do something to signal the driver
/// what happened. This is only meaningful for SerializedDiagnosticConsumers,
/// so here's a placeholder.

virtual void informDriverOfIncompleteBatchModeCompilation() {}
};

/// \brief DiagnosticConsumer that discards all diagnostics.
Expand Down Expand Up @@ -193,13 +202,14 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info) override;

bool finishProcessing(SourceManager &) override;
bool finishProcessing() override;

private:
/// In batch mode, any error causes failure for all primary files, but
/// Xcode will only see an error for a particular primary in that primary's
/// serialized diagnostics file. So, emit errors for all other primaries here.
void addNonSpecificErrors(SourceManager &SM);
/// serialized diagnostics file. So, tell the subconsumers to inform the
/// driver of incomplete batch mode compilation.
void tellSubconsumersToInformDriverOfIncompleteBatchModeCompilation() const;

void computeConsumersOrderedByRange(SourceManager &SM);

Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/DiagnosticEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ namespace swift {

/// \returns true if any diagnostic consumer gave an error while invoking
//// \c finishProcessing.
bool finishProcessing(SourceManager &);
bool finishProcessing();

/// \brief Format the given diagnostic text and place the result in the given
/// buffer.
Expand Down
2 changes: 0 additions & 2 deletions include/swift/AST/DiagnosticsFrontend.def
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,6 @@ WARNING(cannot_assign_value_to_conditional_compilation_flag,none,
ERROR(error_optimization_remark_pattern, none, "%0 in '%1'",
(StringRef, StringRef))

ERROR(error_compilation_stopped_by_errors_in_other_files,none, "compilation stopped by errors in other files", ())

#ifndef DIAG_NO_UNDEF
# if defined(DIAG)
# undef DIAG
Expand Down
30 changes: 7 additions & 23 deletions lib/AST/DiagnosticConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,41 +189,25 @@ void FileSpecificDiagnosticConsumer::handleDiagnostic(
Kind == DiagnosticKind::Error;
}

bool FileSpecificDiagnosticConsumer::finishProcessing(SourceManager &SM) {
addNonSpecificErrors(SM);
bool FileSpecificDiagnosticConsumer::finishProcessing() {
tellSubconsumersToInformDriverOfIncompleteBatchModeCompilation();

// Deliberately don't use std::any_of here because we don't want early-exit
// behavior.

bool hadError = false;
for (auto &subConsumer : SubConsumers)
hadError |= subConsumer.second && subConsumer.second->finishProcessing(SM);
hadError |= subConsumer.second && subConsumer.second->finishProcessing();
return hadError;
}

static void produceNonSpecificError(
FileSpecificDiagnosticConsumer::ConsumerSpecificInformation &info,
SourceManager &SM) {
Diagnostic diagnostic(
diag::error_compilation_stopped_by_errors_in_other_files);

// Stolen from DiagnosticEngine::emitDiagnostic
DiagnosticInfo Info;
Info.ID = diagnostic.getID();

info.consumer->handleDiagnostic(
SM, info.range.getStart(), DiagnosticKind::Error,
DiagnosticEngine::diagnosticStringFor(diagnostic.getID()), {}, Info);
}

void FileSpecificDiagnosticConsumer::addNonSpecificErrors(SourceManager &SM) {
void FileSpecificDiagnosticConsumer::
tellSubconsumersToInformDriverOfIncompleteBatchModeCompilation() const {
if (!HasAnErrorBeenConsumed)
return;
for (auto &info : ConsumersOrderedByRange) {
if (!info.hasAnErrorBeenEmitted && info.consumer) {
produceNonSpecificError(info, SM);
info.hasAnErrorBeenEmitted = true;
}
if (!info.hasAnErrorBeenEmitted && info.consumer)
info.consumer->informDriverOfIncompleteBatchModeCompilation();
}
}

Expand Down
4 changes: 2 additions & 2 deletions lib/AST/DiagnosticEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,10 @@ bool DiagnosticEngine::isDiagnosticPointsToFirstBadToken(DiagID ID) const {
return storedDiagnosticInfos[(unsigned) ID].pointsToFirstBadToken;
}

bool DiagnosticEngine::finishProcessing(SourceManager &SM) {
bool DiagnosticEngine::finishProcessing() {
bool hadError = false;
for (auto &Consumer : Consumers) {
hadError |= Consumer->finishProcessing(SM);
hadError |= Consumer->finishProcessing();
}
return hadError;
}
Expand Down
59 changes: 53 additions & 6 deletions lib/Driver/Compilation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@

#include "CompilationRecord.h"

#include <signal.h>

#define DEBUG_TYPE "batch-mode"

// Batch-mode has a sub-mode for testing that randomizes batch partitions,
Expand Down Expand Up @@ -459,6 +461,35 @@ namespace driver {
}
}

/// Check to see if a job produced a zero-length serialized diagnostics
/// file, which is used to indicate batch-constituents that were batched
/// together with a failing constituent but did not, themselves, produce any
/// errors.
bool jobWasBatchedWithFailingJobs(const Job *J) const {
auto DiaPath =
J->getOutput().getAnyOutputForType(file_types::TY_SerializedDiagnostics);
if (DiaPath.empty())
return false;
if (!llvm::sys::fs::is_regular_file(DiaPath))
return false;
uint64_t Size;
auto EC = llvm::sys::fs::file_size(DiaPath, Size);
if (EC)
return false;
return Size == 0;
}

/// If a batch-constituent job happens to be batched together with a job
/// that exits with an error, the batch-constituent may be considered
/// "cancelled".
bool jobIsCancelledBatchConstituent(int ReturnCode,
const Job *ContainerJob,
const Job *ConstituentJob) {
return ReturnCode != 0 &&
isBatchJob(ContainerJob) &&
jobWasBatchedWithFailingJobs(ConstituentJob);
}

/// Unpack a \c BatchJob that has finished into its constituent \c Job
/// members, and call \c taskFinished on each, propagating any \c
/// TaskFinishedResponse other than \c
Expand All @@ -482,6 +513,26 @@ namespace driver {
return res;
}

void
emitParseableOutputForEachFinishedJob(ProcessId Pid, int ReturnCode,
StringRef Output,
const Job *FinishedCmd) {
FinishedCmd->forEachContainedJobAndPID(Pid, [&](const Job *J,
Job::PID P) {
if (jobIsCancelledBatchConstituent(ReturnCode, FinishedCmd, J)) {
// Simulate SIGINT-interruption to parseable-output consumer for any
// constituent of a failing batch job that produced no errors of its
// own.
parseable_output::emitSignalledMessage(llvm::errs(), *J, P,
"cancelled batch constituent",
"", SIGINT);
} else {
parseable_output::emitFinishedMessage(llvm::errs(), *J, P, ReturnCode,
Output);
}
});
}

/// Callback which will be called immediately after a task has finished
/// execution. Determines if execution should continue, and also schedule
/// any additional Jobs which we now know we need to run.
Expand All @@ -508,12 +559,8 @@ namespace driver {
llvm::errs() << Output;
break;
case OutputLevel::Parseable:
// Parseable output was requested.
FinishedCmd->forEachContainedJobAndPID(
Pid, [&](const Job *J, Job::PID P) {
parseable_output::emitFinishedMessage(llvm::errs(), *J, P,
ReturnCode, Output);
});
emitParseableOutputForEachFinishedJob(Pid, ReturnCode, Output,
FinishedCmd);
break;
}
}
Expand Down
26 changes: 22 additions & 4 deletions lib/Frontend/SerializedDiagnosticConsumer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ class SerializedDiagnosticConsumer : public DiagnosticConsumer {
/// \brief State shared among the various clones of this diagnostic consumer.
llvm::IntrusiveRefCntPtr<SharedState> State;
bool CalledFinishProcessing = false;
bool CompilationWasComplete = true;

public:
SerializedDiagnosticConsumer(StringRef serializedDiagnosticsPath)
: State(new SharedState(serializedDiagnosticsPath)) {
Expand All @@ -140,7 +142,7 @@ class SerializedDiagnosticConsumer : public DiagnosticConsumer {
assert(CalledFinishProcessing && "did not call finishProcessing()");
}

bool finishProcessing(SourceManager &SM) override {
bool finishProcessing() override {
assert(!CalledFinishProcessing &&
"called finishProcessing() multiple times");
CalledFinishProcessing = true;
Expand All @@ -160,19 +162,35 @@ class SerializedDiagnosticConsumer : public DiagnosticConsumer {
llvm::sys::fs::F_None));
if (EC) {
// Create a temporary diagnostics engine to print the error to stderr.
DiagnosticEngine DE(SM);
SourceManager dummyMgr;
DiagnosticEngine DE(dummyMgr);
PrintingDiagnosticConsumer PDC;
DE.addConsumer(PDC);
DE.diagnose(SourceLoc(), diag::cannot_open_serialized_file,
State->SerializedDiagnosticsPath, EC.message());
return true;
}

OS->write((char *)&State->Buffer.front(), State->Buffer.size());
OS->flush();
if (CompilationWasComplete) {
OS->write((char *)&State->Buffer.front(), State->Buffer.size());
OS->flush();
}
return false;
}

/// In batch mode, if any error occurs, no primaries can be compiled.
/// Some primaries will have errors in their diagnostics files and so
/// a client (such as Xcode) can see that those primaries failed.
/// Other primaries will have no errors in their diagnostics files.
/// In order for the driver to distinguish the two cases without parsing
/// the diagnostics, the frontend emits a truncated diagnostics file
/// for the latter case.
/// The unfortunate aspect is that the truncation discards warnings, etc.

void informDriverOfIncompleteBatchModeCompilation() override {
CompilationWasComplete = false;
}

void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
DiagnosticKind Kind,
StringRef FormatString,
Expand Down
4 changes: 2 additions & 2 deletions lib/FrontendTool/FrontendTool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ class JSONFixitWriter
}
}

bool finishProcessing(SourceManager &) override {
bool finishProcessing() override {
std::error_code EC;
std::unique_ptr<llvm::raw_fd_ostream> OS;
OS.reset(new llvm::raw_fd_ostream(FixitsOutputPath,
Expand Down Expand Up @@ -1651,7 +1651,7 @@ int swift::performFrontend(ArrayRef<const char *> Args,

auto finishDiagProcessing = [&](int retValue) -> int {
FinishDiagProcessingCheckRAII.CalledFinishDiagProcessing = true;
bool err = Instance->getDiags().finishProcessing(Instance->getSourceMgr());
bool err = Instance->getDiags().finishProcessing();
return retValue ? retValue : err;
};

Expand Down
11 changes: 11 additions & 0 deletions test/Driver/batch_mode_parseable_output_cancellation.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// RUN: %empty-directory(%t)
// RUN: touch %t/file-01.swift
// RUN: echo 'public func main() { help_an_error_happened() }' >%t/main.swift
//
// RUN: not %swiftc_driver -enable-batch-mode -parseable-output -serialize-diagnostics -c -emit-module -module-name main -j 1 %t/file-01.swift %t/main.swift 2>&1 | %FileCheck %s
//
// CHECK: "kind": "signalled",
// CHECK-NEXT: "name": "compile",
// CHECK-NEXT: "pid": -{{[1-9][0-9]*}},
// CHECK-NEXT: "error-message": "{{.*}}",
// CHECK-NEXT: "signal": 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Ensure that an error in a non-primary causes an error in the errorless primary.
//
// RUN: rm -f %t.*

// RUN: not %target-swift-frontend -typecheck -primary-file %s -serialize-diagnostics-path %t.main.dia -primary-file %S/../Inputs/empty.swift -serialize-diagnostics-path %t.empty.dia %S/Inputs/serialized-diagnostics-batch-mode-suppression-helper.swift 2> %t.stderr.txt

// RUN: test -e %t.main.dia -a ! -s %t.empty.dia
// RUN: test -e %t.empty.dia -a ! -s %t.empty.dia

func test(x: SomeType) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@

// RUN: not %target-swift-frontend -typecheck -primary-file %s -serialize-diagnostics-path %t.main.dia -primary-file %S/../Inputs/empty.swift -serialize-diagnostics-path %t.empty.dia %S/Inputs/serialized-diagnostics-batch-mode-suppression-helper.swift 2> %t.stderr.txt
// RUN: c-index-test -read-diagnostics %t.main.dia 2> %t.main.txt
// RUN: c-index-test -read-diagnostics %t.empty.dia 2> %t.empty.txt

// RUN: %FileCheck -check-prefix=NO-GENERAL-ERROR-OCCURRED %s <%t.main.txt
// RUN: %FileCheck -check-prefix=GENERAL-ERROR-OCCURRED %s <%t.empty.txt
// RUN: test -e %t.empty.dia -a ! -s %t.empty.dia
// GENERAL-ERROR-OCCURRED: compilation stopped by errors in other files
// NO-GENERAL-ERROR-OCCURRED-NOT: compilation stopped by errors in other files

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@

// RUN: not %target-swift-frontend -typecheck -primary-file %s -serialize-diagnostics-path %t.main.dia -primary-file %S/../Inputs/empty.swift -serialize-diagnostics-path %t.empty.dia 2> %t.stderr.txt
// RUN: c-index-test -read-diagnostics %t.main.dia 2> %t.main.txt
// RUN: c-index-test -read-diagnostics %t.empty.dia 2> %t.empty.txt

// RUN: %FileCheck -check-prefix=ERROR %s <%t.main.txt
// RUN: %FileCheck -check-prefix=NO-NONSPECIFIC-ERROR %s <%t.main.txt
// RUN: %FileCheck -check-prefix=NONSPECIFIC-ERROR %s <%t.empty.txt
// RUN: test -e %t.empty.dia -a ! -s %t.empty.dia

// ERROR: error:
// NONSPECIFIC-ERROR: error: compilation stopped by errors in other files
Expand All @@ -17,4 +16,3 @@
func test(x: SomeType) {
nonexistent()
}

Loading