Skip to content

Conversation

@artemcm
Copy link
Contributor

@artemcm artemcm commented Feb 6, 2023

This change causes the dependency scanner to emit complete command-line argument sets for Swift module dependencies that must be built from their .swiftinterface. Previously, the scanner emitted command-line sets which the client (SwiftDriver) modified to make complete by adding:

  • -o <output_path>
  • -disable-implicit-swift-modules
  • -Xcc -fno-implicit-modules and -Xcc -fno-implicit-module-maps
  • -candidate-module-file <module_path>

And, crucially, all of a given dependency module's explicit dependency inputs, via:

  • -explicit-swift-module-map-file for Swift dependencies
  • -Xcc -fmodule-file= and -Xcc -fmodule-map-file= for Clang dependencies

All of the above are now specified by the scanner. This change (re)adds a flag for specifying an explicit module dependency on the command-line, instead of via a JSON file input: -swift-module-file. Module dependencies built from interface files do not require the full functionality provided by explicit module maps, and benefit from having the expanded command-line itself be a complete specification of compilation.

Unlike the prior version of this flag, -swift-module-file no longer requires the specified argument input binary module files to be loaded into memory and opened to determine which module they belong. Instead, it specifies the following format:
-swift-module-file=<module_name>=<module_path>, analogous to Clang's -fmodule-file=<>=<>.

The JSON format of -explicit-swift-module-map-file will continue to be used for source module compile commands, as produced by SwiftDriver.

The full set of dependencies for a given textual Swift module's compile command is produced by computing a transitive closure on the dependency graph, relying on the fact that it is a directed acyclic graph.
The algorithm used is the same one as currently in-use in the driver to do so:

    for each v ∈ V {
        T(v) = { v }
    }
    for v ∈ V in reverse topological order {
        for each (v, w) ∈ E {
             T(v) = T(v) ∪ T(w)
        }
    }

@artemcm
Copy link
Contributor Author

artemcm commented Feb 6, 2023

@swift-ci test

@artemcm artemcm marked this pull request as ready for review February 6, 2023 22:38
Copy link
Contributor

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

Much simpler. Thank you!

@artemcm artemcm force-pushed the ExplicitDependenciesOnCommandLine branch from 3ada2f9 to 032f59a Compare February 6, 2023 23:19
@artemcm
Copy link
Contributor Author

artemcm commented Feb 6, 2023

@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented Feb 7, 2023

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.

Is there any plan to change explicit-swift-module-map-file used by swiftinterface compilation? I didn't check if the order of that is deterministic and I don't really care about if it is command-line or file input but ideally I would like that to be generated by libSwiftScan, not driver so I can compute a cache key during scanning without driver involved.

…dencies on the command line

Do this by computing a transitive closure on the computed dependency graph, relying on the fact that it is a DAG.
The used algorithm is:
```
for each v ∈ V {
    T(v) = { v }
}
for v ∈ V in reverse topological order {
    for each (v, w) ∈ E {
         T(v) = T(v) ∪ T(w)
    }
}
```
… for self-contained commands

- '-o <output_path>'
- '-disable-implicit-swift-modules'
- '-Xcc -fno-implicit-modules' and '-Xcc -fno-implicit-module-maps'
- '-candidate-module-file'

These were previously supplied by the driver. Instead, they will now be ready to be run directly from the dependency scanner's output.
@artemcm artemcm force-pushed the ExplicitDependenciesOnCommandLine branch from 032f59a to be3812d Compare February 7, 2023 18:35
@artemcm
Copy link
Contributor Author

artemcm commented Feb 7, 2023

Is there any plan to change explicit-swift-module-map-file used by swiftinterface compilation? I didn't check if the order of that is deterministic and I don't really care about if it is command-line or file input but ideally I would like that to be generated by libSwiftScan, not driver so I can compute a cache key during scanning without driver involved.

As of this change, swiftinterface dependencies will no longer be using explicit-swift-module-map-file at all, relying on -swift-module-file= on the command-line instead.

We can revisit the format and usage of explicit-swift-module-map-file as it appears on source compile commands in the future.

@artemcm
Copy link
Contributor Author

artemcm commented Feb 7, 2023

@artemcm
Copy link
Contributor Author

artemcm commented Feb 7, 2023

One nice bonus of this change is that the dependency scanner output uses the newly-computed topological ordering of all the modules, which makes it a bit more deterministic for the clients, and nicer to consume by humans in the JSON output.

@artemcm
Copy link
Contributor Author

artemcm commented Feb 7, 2023

As of this change, swiftinterface dependencies will no longer be using explicit-swift-module-map-file at all, relying on -swift-module-file= on the command-line instead.

Wanted to clarify that we will also need swiftlang/swift-driver#1282 for this.

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.

SGTM

@artemcm artemcm merged commit acd72ef into swiftlang:main Feb 8, 2023
@artemcm artemcm deleted the ExplicitDependenciesOnCommandLine branch February 8, 2023 15:25
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.

3 participants