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.

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

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Jul 9, 2024

On dependency scan failure query, ensure any diagnostics that the compiler has accumulated along the way are still produced to the client.

For example, if scan query compilation instance initialization fails on an unknown argument to the frontend, the unknown argument diagnostic will be captured, and returned to the client.

Or, on encountering a dependency cycle, instead of returning an error to the client, a hollow result object is constructed carrying the diagnostic error state.

Resolves rdar://130897498

@artemcm artemcm force-pushed the ScannerFailureMinimalDiagnostics branch 4 times, most recently from 354d402 to c67ee46 Compare July 9, 2024 21:12
@artemcm artemcm requested a review from cachemeifyoucan July 9, 2024 21:13
@artemcm artemcm marked this pull request as ready for review July 9, 2024 21:13
@artemcm
Copy link
Contributor Author

artemcm commented Jul 9, 2024

@swift-ci test

@artemcm artemcm changed the title [Dependency Scanning] Emit diagnostics from compilation instance initialization, even on failure [Dependency Scanning] Emit diagnostics on scan query failure during initialization or cycle detection Jul 9, 2024
Copy link
Contributor

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

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

LGTM in general.

// 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.

@artemcm
Copy link
Contributor Author

artemcm commented Jul 10, 2024

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented Jul 10, 2024

@artemcm artemcm force-pushed the ScannerFailureMinimalDiagnostics branch from c67ee46 to f72d70b Compare July 10, 2024 17:34
@artemcm
Copy link
Contributor Author

artemcm commented Jul 10, 2024

@artemcm artemcm force-pushed the ScannerFailureMinimalDiagnostics branch from f72d70b to a47bf36 Compare July 10, 2024 19:47
@artemcm
Copy link
Contributor Author

artemcm commented Jul 10, 2024

@artemcm artemcm merged commit a5e3154 into swiftlang:main Jul 11, 2024
5 checks passed
@artemcm artemcm deleted the ScannerFailureMinimalDiagnostics branch July 11, 2024 16:08
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