Skip to content
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

[Dependency Scanning] Specify Source Locations For Missing Module Dependencies #73600

Merged
merged 1 commit into from
May 22, 2024

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented May 13, 2024

This change modifies the dependency scanner to keep track of source locations of each encountered import statement, in order to be able to emit diagnostics with source locations if an import failed to resolve.

  • Keep track of each import statement's source buffer, line number, and column number when adding it. The dependency scanner utilizes separate compilation instances, and therefore separate Source Managers for scanning import statements of user sources and textual interfaces of Swift dependencies. Since import resolution may happen in the main scanner compilation instance while the import itself was found by an interface-scanning sub-instance, we cannot simply hold on to the import's SourceLoc.
  • Add libSwiftScan API for diagnostics to carry above source locations to clients.

Instead of unlocatable:

<unknown>0 error: Unable to find module dependency: 'Baz'

we will now get rich output:

/.../test.swift:2:8: error: Unable to find module dependency: 'Baz'
 1 | import A
 2 | import Baz
   |        |- error: Unable to find module dependency: 'Baz'
   |        `- note: a dependency of main module 'test'
 3 | import C
 4 | import Baz
   |        `- note: also imported here

@artemcm artemcm marked this pull request as ready for review May 13, 2024 19:01
@artemcm artemcm force-pushed the DepScanSourceLocsV2 branch 2 times, most recently from 44d745d to 2a7f2ff Compare May 13, 2024 19:07
@artemcm
Copy link
Contributor Author

artemcm commented May 13, 2024

@swift-ci test

// CHECK-NEXT: 2 | // swift-module-flags: -module-name Z
// CHECK-NEXT: 3 | import missing_module
// CHECK-NEXT: | |- error: Unable to find module dependency: 'missing_module'
// CHECK-NEXT: | |- note: a dependency of Swift module 'Z': '{{.*}}{{/|\\}}Z.swiftinterface'
Copy link
Contributor Author

@artemcm artemcm May 13, 2024

Choose a reason for hiding this comment

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

A follow-up change will associate these notes with their corresponding source location of the importer of the given module in the dependency chain.

@artemcm artemcm requested a review from nkcsgexi May 13, 2024 19:22
Copy link
Member

@nkcsgexi nkcsgexi left a comment

Choose a reason for hiding this comment

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

Great enhancement on build logs. Thank you!

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.

Nice. We need to send more diagnostics location back (like if a syntax error in the main source module).

include/swift/DependencyScan/StringUtils.h Outdated Show resolved Hide resolved
tools/libSwiftScan/libSwiftScan.cpp Outdated Show resolved Hide resolved
@artemcm
Copy link
Contributor Author

artemcm commented May 14, 2024

Waiting on apple/llvm-project#8754 to land and auto-merge before this can land.

@artemcm
Copy link
Contributor Author

artemcm commented May 15, 2024

@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented May 15, 2024

@swift-ci test

@xedin xedin removed their request for review May 15, 2024 18:46
@artemcm
Copy link
Contributor Author

artemcm commented May 16, 2024

apple/swift-driver#1614
@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented May 16, 2024

apple/swift-driver#1614
@swift-ci test

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented May 17, 2024

apple/swift-driver#1614
@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented May 17, 2024

apple/swift-driver#1614
@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented May 20, 2024

apple/swift-driver#1614
@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented May 20, 2024

apple/swift-driver#1614
@swift-ci test

1 similar comment
@artemcm
Copy link
Contributor Author

artemcm commented May 20, 2024

apple/swift-driver#1614
@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented May 21, 2024

@swift-ci Please Build Toolchain Ubuntu 22.04

@artemcm
Copy link
Contributor Author

artemcm commented May 21, 2024

@swift-ci Please Build Toolchain Ubuntu 22.04 (arm64)

@artemcm
Copy link
Contributor Author

artemcm commented May 21, 2024

apple/swift-driver#1614
@swift-ci test

…endencies

This change modifies the dependency scanner to keep track of source locations of each encountered 'import' statement, in order to be able to emit diagnostics with source locations if an import failed to resolve.

- Keep track of each 'import' statement's source buffer, line number, and column number when adding it. The dependency scanner utilizes separate compilation instances, and therefore separate Source Managers for scanning `import` statements of user sources and textual interfaces of Swift dependencies. Since import resolution may happen in the main scanner compilation instance while the `import` itself was found by an interface-scanning sub-instance, we cannot simply hold on to the import's `SourceLoc`.
- Add libSwiftScan API for diagnostics to carry above source locations to clients.
@artemcm
Copy link
Contributor Author

artemcm commented May 21, 2024

apple/swift-driver#1614
@swift-ci test

@artemcm artemcm merged commit 0b9c25d into apple:main May 22, 2024
5 checks passed
@artemcm artemcm deleted the DepScanSourceLocsV2 branch May 22, 2024 04:14
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.

None yet

4 participants