Skip to content

[6.2 🍒][Dependency Scanning] Always use a locking diagnostic consumer #81294

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
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
22 changes: 1 addition & 21 deletions include/swift/DependencyScan/DependencyScanningTool.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class DependencyScanDiagnosticCollector : public DiagnosticConsumer {
std::optional<ScannerImportStatementInfo::ImportDiagnosticLocationInfo> ImportLocation;
};
std::vector<ScannerDiagnosticInfo> Diagnostics;
llvm::sys::SmartMutex<true> ScanningDiagnosticConsumerStateLock;

void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;

Expand All @@ -55,19 +56,6 @@ class DependencyScanDiagnosticCollector : public DiagnosticConsumer {
}
};

/// Locking variant of the above diagnostic collector that guards accesses to
/// its state with a lock.
class LockingDependencyScanDiagnosticCollector
: public DependencyScanDiagnosticCollector {
private:
void addDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;
llvm::sys::SmartMutex<true> ScanningDiagnosticConsumerStateLock;

public:
friend DependencyScanningTool;
LockingDependencyScanDiagnosticCollector() {}
};

/// Given a set of arguments to a print-target-info frontend tool query, produce the
/// JSON target info.
llvm::ErrorOr<swiftscan_string_ref_t> getTargetInfo(ArrayRef<const char *> Command,
Expand Down Expand Up @@ -97,11 +85,6 @@ class DependencyScanningTool {
llvm::ErrorOr<swiftscan_import_set_t>
getImports(ArrayRef<const char *> Command, StringRef WorkingDirectory);

/// Query diagnostics consumed so far.
std::vector<DependencyScanDiagnosticCollector::ScannerDiagnosticInfo> getDiagnostics();
/// Discared the collection of diagnostics encountered so far.
void resetDiagnostics();

/// Using the specified invocation command, instantiate a CompilerInstance
/// that will be used for this scan.
llvm::ErrorOr<ScanQueryInstance>
Expand All @@ -116,9 +99,6 @@ class DependencyScanningTool {

/// Shared state mutual-exclusivity lock
llvm::sys::SmartMutex<true> DependencyScanningToolStateLock;

/// A shared consumer that accumulates encountered diagnostics.
LockingDependencyScanDiagnosticCollector CDC;
llvm::BumpPtrAllocator Alloc;
llvm::StringSaver Saver;
};
Expand Down
29 changes: 5 additions & 24 deletions lib/DependencyScan/DependencyScanningTool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ void DependencyScanDiagnosticCollector::handleDiagnostic(SourceManager &SM,

void DependencyScanDiagnosticCollector::addDiagnostic(
SourceManager &SM, const DiagnosticInfo &Info) {
llvm::sys::SmartScopedLock<true> Lock(ScanningDiagnosticConsumerStateLock);

// Determine what kind of diagnostic we're emitting.
llvm::SourceMgr::DiagKind SMKind;
switch (Info.Kind) {
Expand Down Expand Up @@ -129,12 +131,6 @@ void DependencyScanDiagnosticCollector::addDiagnostic(
}
}

void LockingDependencyScanDiagnosticCollector::addDiagnostic(
SourceManager &SM, const DiagnosticInfo &Info) {
llvm::sys::SmartScopedLock<true> Lock(ScanningDiagnosticConsumerStateLock);
DependencyScanDiagnosticCollector::addDiagnostic(SM, Info);
}

swiftscan_diagnostic_set_t *mapCollectedDiagnosticsForOutput(
const DependencyScanDiagnosticCollector *diagnosticCollector) {
auto collectedDiagnostics = diagnosticCollector->getDiagnostics();
Expand Down Expand Up @@ -263,7 +259,7 @@ static swiftscan_import_set_t generateHollowDiagnosticOutputImportSet(

DependencyScanningTool::DependencyScanningTool()
: ScanningService(std::make_unique<SwiftDependencyScanningService>()),
CDC(), Alloc(), Saver(Alloc) {}
Alloc(), Saver(Alloc) {}

llvm::ErrorOr<swiftscan_dependency_graph_t>
DependencyScanningTool::getDependencies(
Expand All @@ -272,7 +268,8 @@ DependencyScanningTool::getDependencies(
StringRef WorkingDirectory) {
// There may be errors as early as in instance initialization, so we must ensure
// we can catch those.
auto ScanDiagnosticConsumer = std::make_shared<DependencyScanDiagnosticCollector>();
auto ScanDiagnosticConsumer =
std::make_shared<DependencyScanDiagnosticCollector>();

// The primary instance used to scan the query Swift source-code
auto QueryContextOrErr = initCompilerInstanceForScan(Command,
Expand Down Expand Up @@ -329,18 +326,6 @@ DependencyScanningTool::getImports(ArrayRef<const char *> Command,
return std::move(*DependenciesOrErr);
}

std::vector<
DependencyScanDiagnosticCollector::ScannerDiagnosticInfo>
DependencyScanningTool::getDiagnostics() {
llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
return CDC.Diagnostics;
}

void DependencyScanningTool::resetDiagnostics() {
llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
CDC.reset();
}

llvm::ErrorOr<ScanQueryInstance>
DependencyScanningTool::initCompilerInstanceForScan(
ArrayRef<const char *> CommandArgs,
Expand All @@ -356,10 +341,6 @@ DependencyScanningTool::initCompilerInstanceForScan(

// State unique to an individual scan
auto Instance = std::make_unique<CompilerInstance>();

// FIXME: The shared CDC must be deprecated once all clients have switched
// to using per-scan diagnostic output embedded in the `swiftscan_dependency_graph_s`
Instance->addDiagnosticConsumer(&CDC);
Instance->addDiagnosticConsumer(scannerDiagnosticsCollector.get());

// Basic error checking on the arguments
Expand Down
40 changes: 5 additions & 35 deletions lib/Tooling/libSwiftScan/libSwiftScan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -589,45 +589,15 @@ swiftscan_compiler_supported_features_query() {
//=== Scanner Diagnostics -------------------------------------------------===//
swiftscan_diagnostic_set_t*
swiftscan_scanner_diagnostics_query(swiftscan_scanner_t scanner) {
DependencyScanningTool *ScanningTool = unwrap(scanner);
auto Diagnostics = ScanningTool->getDiagnostics();
auto NumDiagnostics = Diagnostics.size();

swiftscan_diagnostic_set_t *Result = new swiftscan_diagnostic_set_t;
Result->count = NumDiagnostics;
Result->diagnostics = new swiftscan_diagnostic_info_t[NumDiagnostics];

for (size_t i = 0; i < NumDiagnostics; ++i) {
const auto &Diagnostic = Diagnostics[i];
swiftscan_diagnostic_info_s *DiagnosticInfo = new swiftscan_diagnostic_info_s;
DiagnosticInfo->message = swift::c_string_utils::create_clone(Diagnostic.Message.c_str());
switch (Diagnostic.Severity) {
case llvm::SourceMgr::DK_Error:
DiagnosticInfo->severity = SWIFTSCAN_DIAGNOSTIC_SEVERITY_ERROR;
break;
case llvm::SourceMgr::DK_Warning:
DiagnosticInfo->severity = SWIFTSCAN_DIAGNOSTIC_SEVERITY_WARNING;
break;
case llvm::SourceMgr::DK_Note:
DiagnosticInfo->severity = SWIFTSCAN_DIAGNOSTIC_SEVERITY_NOTE;
break;
case llvm::SourceMgr::DK_Remark:
DiagnosticInfo->severity = SWIFTSCAN_DIAGNOSTIC_SEVERITY_REMARK;
break;
}
// swiftscan_scanner_diagnostics_query is deprecated,
// so it does not support source locations.
DiagnosticInfo->source_location = nullptr;
Result->diagnostics[i] = DiagnosticInfo;
}

return Result;
// This method is deprecated
swiftscan_diagnostic_set_t *set = new swiftscan_diagnostic_set_t;
set->count = 0;
return set;
}

void
swiftscan_scanner_diagnostics_reset(swiftscan_scanner_t scanner) {
DependencyScanningTool *ScanningTool = unwrap(scanner);
ScanningTool->resetDiagnostics();
// This method is deprecated
}

swiftscan_string_ref_t
Expand Down