Skip to content

[cxx-interop] Import more C++ source locations into Swift #77204

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 1 commit into from
Oct 31, 2024

Conversation

Xazax-hun
Copy link
Contributor

Occasionally, when the Swift compiler emits a diagnostic for a construct that was imported from C++ we get a diagnostic with unknown location. This is a bad user experience. It is particularly bad with the borrow-checker related diagnostics. This patch extends the source location importing to declarations in ClangImporter. There are some invariants enforced by the Swift compile, e.g., a source range is comprised of two valid source locations or two invalid ones. As a result, this patch adds approximate source locations to some separators like braces or parens that are not maintained by Clang. Having slightly incorrect ranges in this case is better than emitting unknown source locations.

@Xazax-hun Xazax-hun added the c++ interop Feature: Interoperability with C++ label Oct 24, 2024
@Xazax-hun
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

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

LGTM, just one minor question.

@Xazax-hun Xazax-hun force-pushed the gaborh/diagnostic-location branch 2 times, most recently from 1b26266 to 7492921 Compare October 25, 2024 16:01
@Xazax-hun
Copy link
Contributor Author

@swift-ci please test

@Xazax-hun Xazax-hun force-pushed the gaborh/diagnostic-location branch from 7492921 to e1d8a23 Compare October 28, 2024 13:41
@Xazax-hun
Copy link
Contributor Author

@swift-ci please test

Occasionally, when the Swift compiler emits a diagnostic for a construct
that was imported from C++ we get a diagnostic with unknown location.
This is a bad user experience. It is particularly bad with the
borrow-checker related diagnostics. This patch extends the source
location importing to declarations in ClangImporter. There are some
invariants enforced by the Swift compile, e.g., a source range is
comprised of two valid source locations or two invalid ones. As a
result, this patch adds approximate source locations to some separators
like braces or parens that are not maintained by Clang. Having slightly
incorrect ranges in this case is better than emitting unknown source
locations.
@Xazax-hun
Copy link
Contributor Author

@swift-ci please smoke test linux

@Xazax-hun Xazax-hun force-pushed the gaborh/diagnostic-location branch from e1d8a23 to 86e708a Compare October 30, 2024 13:34
@Xazax-hun
Copy link
Contributor Author

@swift-ci please test

@Xazax-hun Xazax-hun merged commit 09c89fb into main Oct 31, 2024
5 checks passed
@Xazax-hun Xazax-hun deleted the gaborh/diagnostic-location branch October 31, 2024 11:06
Xazax-hun pushed a commit that referenced this pull request Oct 31, 2024
A recent PR (#77204) started to import C++ source locations into Swift.
This PR flips a switch so these locations are actually used more widely.
Xazax-hun pushed a commit that referenced this pull request Nov 1, 2024
A recent PR (#77204) started to import C++ source locations into Swift.
This PR flips a switch so these locations are actually used more widely.
Now some of the diagnostic locations are changed, but they generally
improved the quality of the diagnostics, pointing out conformances
imported from Obj-C code right when they are declared.
Xazax-hun pushed a commit that referenced this pull request Nov 1, 2024
A recent PR (#77204) started to import C++ source locations into Swift.
This PR flips a switch so these locations are actually used more widely.
Now some of the diagnostic locations are changed, but they generally
improved the quality of the diagnostics, pointing out conformances
imported from Obj-C code right when they are declared.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants