Skip to content

[Dependency Scanning] Emit diagnostics on scan query failure during initialization or cycle detection #75111

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.

Sign up for GitHub

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 2 commits into from
Jul 11, 2024
Merged

[Dependency Scanning] Emit diagnostics on scan query failure during initialization or cycle detection #75111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4b7bf3a
[Dependency Scanning] Emit diagnostics from compilation instance init…
artemcm Jun 17, 2024
a47bf36
[Dependency Scanning] Construct a hollow output on query failure to c…
artemcm Jul 9, 2024
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
13 changes: 5 additions & 8 deletions include/swift/DependencyScan/DependencyScanningTool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class DependencyScanDiagnosticCollector;

struct ScanQueryInstance {
std::unique_ptr<CompilerInstance> ScanInstance;
std::unique_ptr<DependencyScanDiagnosticCollector> ScanDiagnostics;
std::shared_ptr<DependencyScanDiagnosticCollector> ScanDiagnostics;
};

/// Diagnostic consumer that simply collects the diagnostics emitted so-far
Expand Down Expand Up @@ -124,15 +124,10 @@ class DependencyScanningTool {
/// that will be used for this scan.
llvm::ErrorOr<ScanQueryInstance>
initCompilerInstanceForScan(ArrayRef<const char *> Command,
StringRef WorkingDirectory);
StringRef WorkingDirectory,
std::shared_ptr<DependencyScanDiagnosticCollector> scannerDiagnosticsCollector);

private:
/// Using the specified invocation command, initialize the scanner instance
/// for this scan. Returns the `CompilerInstance` that will be used.
llvm::ErrorOr<ScanQueryInstance>
initScannerForAction(ArrayRef<const char *> Command,
StringRef WorkingDirectory);

/// Shared cache of module dependencies, re-used by individual full-scan queries
/// during the lifetime of this Tool.
std::unique_ptr<SwiftDependencyScanningService> ScanningService;
Expand All @@ -150,6 +145,8 @@ class DependencyScanningTool {
llvm::StringSaver Saver;
};

swiftscan_diagnostic_set_t *mapCollectedDiagnosticsForOutput(const DependencyScanDiagnosticCollector *diagnosticCollector);

} // end namespace dependencies
} // end namespace swift

Expand Down
191 changes: 167 additions & 24 deletions lib/DependencyScan/DependencyScanningTool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,132 @@ void LockingDependencyScanDiagnosticCollector::addDiagnostic(
DependencyScanDiagnosticCollector::addDiagnostic(SM, Info);
}

swiftscan_diagnostic_set_t *mapCollectedDiagnosticsForOutput(
const DependencyScanDiagnosticCollector *diagnosticCollector) {
auto collectedDiagnostics = diagnosticCollector->getDiagnostics();
auto numDiagnostics = collectedDiagnostics.size();
swiftscan_diagnostic_set_t *diagnosticOutput = new swiftscan_diagnostic_set_t;
diagnosticOutput->count = numDiagnostics;
diagnosticOutput->diagnostics =
new swiftscan_diagnostic_info_t[numDiagnostics];
for (size_t i = 0; i < numDiagnostics; ++i) {
const auto &Diagnostic = collectedDiagnostics[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;
}

if (Diagnostic.ImportLocation.has_value()) {
auto importLocation = Diagnostic.ImportLocation.value();
swiftscan_source_location_s *sourceLoc = new swiftscan_source_location_s;
if (importLocation.bufferIdentifier.empty())
sourceLoc->buffer_identifier = swift::c_string_utils::create_null();
else
sourceLoc->buffer_identifier = swift::c_string_utils::create_clone(
importLocation.bufferIdentifier.c_str());
sourceLoc->line_number = importLocation.lineNumber;
sourceLoc->column_number = importLocation.columnNumber;
diagnosticInfo->source_location = sourceLoc;
} else {
diagnosticInfo->source_location = nullptr;
}

diagnosticOutput->diagnostics[i] = diagnosticInfo;
}
return diagnosticOutput;
}

// Generate an instance of the `swiftscan_dependency_graph_s` which contains no
// module dependnecies but captures the diagnostics emitted during the attempted
// scan query.
static swiftscan_dependency_graph_t generateHollowDiagnosticOutput(
Copy link
Contributor

Choose a reason for hiding this comment

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

will this be easier and cleaner to do by constructing an empty ModuleDependencyInfo then convert that into scanner object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code for creating an swiftscan_dependency_info_t from a ModuleDependencyInfo now relies on a ready-populated scanner cache, for example, and a successfully-constructed compilation instance, along with other assumptions, in generateFullDependencyGraph.

It can certainly be refactored to support this use-case as well, and it'd probably be overall cleaner, but I wanted to restrict this change in scope to minimally affect the non-error code-path, since I'd like to get this change into 6.0.

I think this may be a good follow-up change to do on main.

const DependencyScanDiagnosticCollector &ScanDiagnosticConsumer) {
// Create a dependency graph instance
swiftscan_dependency_graph_t hollowResult = new swiftscan_dependency_graph_s;

// Populate the `modules` with a single info for the main module
// containing no dependencies
swiftscan_dependency_set_t *dependencySet = new swiftscan_dependency_set_t;
dependencySet->count = 1;
dependencySet->modules = new swiftscan_dependency_info_t[1];
swiftscan_dependency_info_s *hollowMainModuleInfo =
new swiftscan_dependency_info_s;
dependencySet->modules[0] = hollowMainModuleInfo;
hollowResult->dependencies = dependencySet;

// Other main module details empty
hollowMainModuleInfo->direct_dependencies =
c_string_utils::create_empty_set();
hollowMainModuleInfo->source_files = c_string_utils::create_empty_set();
hollowMainModuleInfo->module_path = c_string_utils::create_null();
hollowResult->main_module_name = c_string_utils::create_clone("unknown");
hollowMainModuleInfo->module_name =
c_string_utils::create_clone("swiftTextual:unknown");

// Hollow info details
swiftscan_module_details_s *hollowDetails = new swiftscan_module_details_s;
hollowDetails->kind = SWIFTSCAN_DEPENDENCY_INFO_SWIFT_TEXTUAL;
swiftscan_macro_dependency_set_t *hollowMacroSet = new swiftscan_macro_dependency_set_t;
hollowMacroSet->count = 0;
hollowMacroSet->macro_dependencies = nullptr;
hollowDetails->swift_textual_details = {c_string_utils::create_null(),
c_string_utils::create_empty_set(),
c_string_utils::create_null(),
c_string_utils::create_empty_set(),
c_string_utils::create_empty_set(),
c_string_utils::create_empty_set(),
c_string_utils::create_empty_set(),
c_string_utils::create_empty_set(),
c_string_utils::create_empty_set(),
c_string_utils::create_null(),
false,
false,
c_string_utils::create_null(),
c_string_utils::create_null(),
c_string_utils::create_null(),
hollowMacroSet};
hollowMainModuleInfo->details = hollowDetails;

// Empty Link Library set
swiftscan_link_library_set_t *hollowLinkLibrarySet =
new swiftscan_link_library_set_t;
hollowLinkLibrarySet->count = 0;
hollowLinkLibrarySet->link_libraries = nullptr;
hollowMainModuleInfo->link_libraries = hollowLinkLibrarySet;

// Populate the diagnostic info
hollowResult->diagnostics =
mapCollectedDiagnosticsForOutput(&ScanDiagnosticConsumer);
return hollowResult;
}

// Generate an instance of the `swiftscan_import_set_t` which contains no
// imports but captures the diagnostics emitted during the attempted
// scan query.
static swiftscan_import_set_t generateHollowDiagnosticOutputImportSet(
const DependencyScanDiagnosticCollector &ScanDiagnosticConsumer) {
// Create an dependency graph instance
swiftscan_import_set_t hollowResult = new swiftscan_import_set_s;
hollowResult->imports = c_string_utils::create_empty_set();
hollowResult->diagnostics =
mapCollectedDiagnosticsForOutput(&ScanDiagnosticConsumer);
return hollowResult;
}

DependencyScanningTool::DependencyScanningTool()
: ScanningService(std::make_unique<SwiftDependencyScanningService>()),
VersionedPCMInstanceCacheCache(
Expand All @@ -148,12 +274,20 @@ DependencyScanningTool::DependencyScanningTool()

llvm::ErrorOr<swiftscan_dependency_graph_t>
DependencyScanningTool::getDependencies(
ArrayRef<const char *> Command, const llvm::StringSet<> &PlaceholderModules,
ArrayRef<const char *> Command,
const llvm::StringSet<> &PlaceholderModules,
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>();

// The primary instance used to scan the query Swift source-code
auto QueryContextOrErr = initScannerForAction(Command, WorkingDirectory);
if (std::error_code EC = QueryContextOrErr.getError())
return EC;
auto QueryContextOrErr = initCompilerInstanceForScan(Command,
WorkingDirectory,
ScanDiagnosticConsumer);
if (QueryContextOrErr.getError())
return generateHollowDiagnosticOutput(*ScanDiagnosticConsumer);

auto QueryContext = std::move(*QueryContextOrErr);

// Local scan cache instance, wrapping the shared global cache.
Expand All @@ -166,19 +300,24 @@ DependencyScanningTool::getDependencies(
QueryContext.ScanDiagnostics.get(),
cache);
if (DependenciesOrErr.getError())
return std::make_error_code(std::errc::not_supported);
auto Dependencies = std::move(*DependenciesOrErr);
return generateHollowDiagnosticOutput(*ScanDiagnosticConsumer);

return Dependencies;
return std::move(*DependenciesOrErr);
}

llvm::ErrorOr<swiftscan_import_set_t>
DependencyScanningTool::getImports(ArrayRef<const char *> Command,
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>();
// The primary instance used to scan the query Swift source-code
auto QueryContextOrErr = initScannerForAction(Command, WorkingDirectory);
if (std::error_code EC = QueryContextOrErr.getError())
return EC;
auto QueryContextOrErr = initCompilerInstanceForScan(Command,
WorkingDirectory,
ScanDiagnosticConsumer);
if (QueryContextOrErr.getError())
return generateHollowDiagnosticOutputImportSet(*ScanDiagnosticConsumer);

auto QueryContext = std::move(*QueryContextOrErr);

// Local scan cache instance, wrapping the shared global cache.
Expand All @@ -190,10 +329,9 @@ DependencyScanningTool::getImports(ArrayRef<const char *> Command,
QueryContext.ScanDiagnostics.get(),
cache);
if (DependenciesOrErr.getError())
return std::make_error_code(std::errc::not_supported);
auto Dependencies = std::move(*DependenciesOrErr);
return generateHollowDiagnosticOutputImportSet(*ScanDiagnosticConsumer);

return Dependencies;
return std::move(*DependenciesOrErr);
}

std::vector<llvm::ErrorOr<swiftscan_dependency_graph_t>>
Expand All @@ -202,10 +340,14 @@ DependencyScanningTool::getDependencies(
const std::vector<BatchScanInput> &BatchInput,
const llvm::StringSet<> &PlaceholderModules, StringRef WorkingDirectory) {
// The primary instance used to scan Swift modules
auto QueryContextOrErr = initScannerForAction(Command, WorkingDirectory);
auto ScanDiagnosticConsumer = std::make_shared<DependencyScanDiagnosticCollector>();
auto QueryContextOrErr = initCompilerInstanceForScan(Command,
WorkingDirectory,
ScanDiagnosticConsumer);
if (std::error_code EC = QueryContextOrErr.getError())
return std::vector<llvm::ErrorOr<swiftscan_dependency_graph_t>>(
BatchInput.size(), std::make_error_code(std::errc::invalid_argument));

auto QueryContext = std::move(*QueryContextOrErr);

// Local scan cache instance, wrapping the shared global cache.
Expand Down Expand Up @@ -264,26 +406,26 @@ void DependencyScanningTool::resetDiagnostics() {
}

llvm::ErrorOr<ScanQueryInstance>
DependencyScanningTool::initScannerForAction(
ArrayRef<const char *> Command, StringRef WorkingDirectory) {
DependencyScanningTool::initCompilerInstanceForScan(
ArrayRef<const char *> CommandArgs,
StringRef WorkingDir,
std::shared_ptr<DependencyScanDiagnosticCollector> scannerDiagnosticsCollector) {
// The remainder of this method operates on shared state in the
// scanning service and global LLVM state with:
// llvm::cl::ResetAllOptionOccurrences
llvm::sys::SmartScopedLock<true> Lock(DependencyScanningToolStateLock);
return initCompilerInstanceForScan(Command, WorkingDirectory);
}
// FIXME: Instead, target-info and supported-features queries must use
// `DependencyScanningToolStateLock`, but this currently requires further
// client-side API plumbing.
llvm::sys::SmartScopedLock<true> TargetInfoLock(TargetInfoMutex);

llvm::ErrorOr<ScanQueryInstance>
DependencyScanningTool::initCompilerInstanceForScan(
ArrayRef<const char *> CommandArgs, StringRef WorkingDir) {
// State unique to an individual scan
auto Instance = std::make_unique<CompilerInstance>();
auto ScanDiagnosticConsumer = std::make_unique<DependencyScanDiagnosticCollector>();

// 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(ScanDiagnosticConsumer.get());
Instance->addDiagnosticConsumer(scannerDiagnosticsCollector.get());

// Basic error checking on the arguments
if (CommandArgs.empty()) {
Expand Down Expand Up @@ -327,6 +469,7 @@ DependencyScanningTool::initCompilerInstanceForScan(
if (Instance->setup(Invocation, InstanceSetupError)) {
return std::make_error_code(std::errc::not_supported);
}
Invocation.getFrontendOptions().LLVMArgs.clear();

// Setup the caching service after the instance finishes setup.
if (ScanningService->setupCachingDependencyScanningService(*Instance))
Expand All @@ -335,7 +478,7 @@ DependencyScanningTool::initCompilerInstanceForScan(
(void)Instance->getMainModule();

return ScanQueryInstance{std::move(Instance),
std::move(ScanDiagnosticConsumer)};
scannerDiagnosticsCollector};
}

} // namespace dependencies
Expand Down
2 changes: 2 additions & 0 deletions lib/DependencyScan/ModuleDependencyScanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ ModuleDependencyScanningWorker::ModuleDependencyScanningWorker(
ScanASTContext.getModuleInterfaceChecker()),
&DependencyTracker,
ScanCompilerInvocation.getSearchPathOptions().ModuleLoadMode);

llvm::cl::ResetAllOptionOccurrences();
}

ModuleDependencyVector
Expand Down
59 changes: 3 additions & 56 deletions lib/DependencyScan/ScanDependencies.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,56 +589,6 @@ static void bridgeDependencyIDs(const ArrayRef<ModuleDependencyID> dependencies,
}
}

static swiftscan_diagnostic_set_t *mapCollectedDiagnosticsForOutput(
const SourceManager &SM,
const DependencyScanDiagnosticCollector *diagnosticCollector) {
auto collectedDiagnostics = diagnosticCollector->getDiagnostics();
auto numDiagnostics = collectedDiagnostics.size();
swiftscan_diagnostic_set_t *diagnosticOutput = new swiftscan_diagnostic_set_t;
diagnosticOutput->count = numDiagnostics;
diagnosticOutput->diagnostics =
new swiftscan_diagnostic_info_t[numDiagnostics];
for (size_t i = 0; i < numDiagnostics; ++i) {
const auto &Diagnostic = collectedDiagnostics[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;
}

if (Diagnostic.ImportLocation.has_value()) {
auto importLocation = Diagnostic.ImportLocation.value();
swiftscan_source_location_s *sourceLoc = new swiftscan_source_location_s;
if (importLocation.bufferIdentifier.empty())
sourceLoc->buffer_identifier = swift::c_string_utils::create_null();
else
sourceLoc->buffer_identifier = swift::c_string_utils::create_clone(
importLocation.bufferIdentifier.c_str());
sourceLoc->line_number = importLocation.lineNumber;
sourceLoc->column_number = importLocation.columnNumber;
diagnosticInfo->source_location = sourceLoc;
} else {
diagnosticInfo->source_location = nullptr;
}

diagnosticOutput->diagnostics[i] = diagnosticInfo;
}
return diagnosticOutput;
}

static swiftscan_macro_dependency_set_t *createMacroDependencySet(
const std::map<std::string, MacroPluginDependency> &macroDeps) {
swiftscan_macro_dependency_set_t *set = new swiftscan_macro_dependency_set_t;
Expand Down Expand Up @@ -867,8 +817,7 @@ generateFullDependencyGraph(const CompilerInstance &instance,
result->dependencies = dependencySet;
result->diagnostics =
diagnosticCollector
? mapCollectedDiagnosticsForOutput(instance.getSourceMgr(),
diagnosticCollector)
? mapCollectedDiagnosticsForOutput(diagnosticCollector)
: nullptr;
return result;
}
Expand Down Expand Up @@ -1574,13 +1523,11 @@ swift::dependencies::performModulePrescan(CompilerInstance &instance,
importSet->imports = create_set(importIdentifiers);
importSet->diagnostics =
diagnosticCollector
? mapCollectedDiagnosticsForOutput(instance.getSourceMgr(),
diagnosticCollector)
? mapCollectedDiagnosticsForOutput(diagnosticCollector)
: nullptr;
importSet->diagnostics =
diagnosticCollector
? mapCollectedDiagnosticsForOutput(instance.getSourceMgr(),
diagnosticCollector)
? mapCollectedDiagnosticsForOutput(diagnosticCollector)
: nullptr;
return importSet;
}
Expand Down
Loading