Skip to content

Conversation

sina-mahdavi
Copy link
Contributor

This PR adds support for the new Clang -header-include-format=direct-per-file output, which now tracks Clang modules. The logic should still support the old format for backwards compatibility if it doesn't detect a version number in the trace file. The related LLVM PR is: llvm/llvm-project#156756

@sina-mahdavi
Copy link
Contributor Author

@swift-ci Please test

@sina-mahdavi sina-mahdavi force-pushed the sina-mahdavi/clang-modules-dependency-info branch from 99d122c to 908f676 Compare September 5, 2025 02:25
@sina-mahdavi
Copy link
Contributor Author

@swift-ci Please test

@@ -542,7 +542,7 @@ fileprivate struct DependencyValidationTests: CoreBasedTests {

// Expect complaint about undeclared dependency
try await tester.checkBuild(parameters: BuildParameters(configuration: "Debug"), runDestination: .host, persistent: true) { results in
results.checkError(.contains("Missing entries in MODULE_DEPENDENCIES: Foundation (for task"))
results.checkError(.contains("Missing entry in MODULE_DEPENDENCIES: Foundation (for task"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm why would this change? I thought the top level diag was always pluralized.

Copy link
Contributor Author

@sina-mahdavi sina-mahdavi Sep 5, 2025

Choose a reason for hiding this comment

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

The test fails for me locally with "entries", I think it's because the "(for task" substring is at the end of the diagnostic which will have the singular messages. So the whole diagnostic is like:

Missing entries in MODULE_DEPENDENCIES: Foundation Foundation
/private/var/folders/5h/9y9ls7ms0fs8gdmlfwklkwpm0000gn/T/swbuild.tmp.x8Tuqtfi/Data.noindex/Test/aProject/Sources/CoreFoo.m: Missing entry in MODULE_DEPENDENCIES: Foundation
/private/var/folders/5h/9y9ls7ms0fs8gdmlfwklkwpm0000gn/T/swbuild.tmp.x8Tuqtfi/Data.noindex/Test/aProject/Sources/CoreBar.m: Missing entry in MODULE_DEPENDENCIES: Foundation (for task: [\"ValidateDependencies\"])

If you want I could change this to look for Missing entries in MODULE_DEPENDENCIES: Foundation Foundation\n?

Copy link
Contributor

Choose a reason for hiding this comment

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

The missing entries are supposed to be de-duplicated (which is why the test checks for "(for task" at the end). It sounds like that's not working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I think there's an issue with the deduplication in my implementation of parsing v1 traces that I need to fix. However, even after fixing that the diagnostic looks like this:

Missing entries in MODULE_DEPENDENCIES: Foundation
Missing entry in MODULE_DEPENDENCIES: Foundation (for task: [\"ValidateDependencies\"])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I think my last commit should fix the deduplication issue, the first line of the diagnostic has only one mention of Foundation. However, Foundation (for task is still only in the last line which has singular entry.

@@ -542,7 +542,7 @@ fileprivate struct DependencyValidationTests: CoreBasedTests {

// Expect complaint about undeclared dependency
try await tester.checkBuild(parameters: BuildParameters(configuration: "Debug"), runDestination: .host, persistent: true) { results in
results.checkError(.contains("Missing entries in MODULE_DEPENDENCIES: Foundation (for task"))
results.checkError(.contains("Missing entry in MODULE_DEPENDENCIES: Foundation (for task"))
Copy link
Contributor

Choose a reason for hiding this comment

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

The missing entries are supposed to be de-duplicated (which is why the test checks for "(for task" at the end). It sounds like that's not working.

@owenv
Copy link
Collaborator

owenv commented Sep 5, 2025

@swift-ci test

1 similar comment
@sina-mahdavi
Copy link
Contributor Author

@swift-ci test

@bob-wilson bob-wilson force-pushed the sina-mahdavi/clang-modules-dependency-info branch from b06bce2 to 6a44f6a Compare September 11, 2025 19:38
@bob-wilson
Copy link
Contributor

@swift-ci test

@bob-wilson bob-wilson merged commit c4d33bf into swiftlang:main Sep 12, 2025
36 of 38 checks passed
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.

6 participants