Implement -debug-module-path for implicit modules in swift-driver#2062
Implement -debug-module-path for implicit modules in swift-driver#2062adrian-prantl wants to merge 1 commit intomainfrom
Conversation
adrian-prantl
requested review from
DougGregor,
artemcm,
cachemeifyoucan,
compnerd and
owenv
as code owners
|
@swift-ci test |
| .parentDirectory.appending(component: moduleOutputInfo.name) | ||
| .replacingExtension(with: .swiftModule) | ||
| .intern() | ||
| try addPathOption(option: .debugModulePath, path: VirtualPath.lookup(path), to: &commandLine, remap: jobNeedPathRemap) |
There was a problem hiding this comment.
There is no more module merge option anymore (new driver doesn't support it).
Is it possible to pass in the swiftmodule path to the function so it is not derived again?
There was a problem hiding this comment.
is it possible to pass in the swiftmodule path to the function so it is not derived again?
I would love to hear some advice for how to better implement this from someone more familiar with swift-driver :-)
There was a problem hiding this comment.
I am not too familiar with how new swift-driver construct implicit module job but I don't think it will construct merge job anymore. forObject is probably still valid for when this flag is needed for implicit build, but you want to double check.
I also think the correct way to get module output path is to directly access self.moduleOuputInfo.output.outputPath, maybe even for explicit module build. But that seems to have a variant module output path for catalyst output. I would like to see a test case for correctly writing -debug-module-path when planning zippered module. (retroactively added for explicit module, for implicit module if that is supported in new driver)
adrian-prantl
force-pushed
the
eng/PR-168256846
branch
from
843aea1 to
4986256
Compare
|
@swift-ci test |
| let mainModule = explicitModulePlanner.dependencyGraph.mainModule | ||
| modulePathHandle = moduleOutputInfo.output?.outputPath ?? mainModule.modulePath.path | ||
| } else { | ||
| // Recompute the module path based on the module name, because this is effectively passing the output |
There was a problem hiding this comment.
Similarly to what @cachemeifyoucan is saying, I do not think we should be computing this here at all. I think this code should rely on moduleOutputInfo.output?.outputPath for both implicit and explicit module code paths, without trying to figure out what it would be if it is nil.
Also, as already mentioned, it would be great to have a test for this which includes -experimental-emit-variant-module -emit-variant-module-path <...>.
There was a problem hiding this comment.
Let me see if I can figure out how to get the path into moduleOutputInfo.output?.outputPath then.
There was a problem hiding this comment.
I uploaded a much simplified version of the patch that unifies the explicit and implicit code paths.
One thing I couldn't resolve is switching to use moduleOutputInfo.output?.outputPath: it contains the unmerged module path (i.e., "Foo-1.swiftmodule") instead of the final merged module name. If you have any ideas for how to fix that, I'm happy to rework this further. IIUC, the main problem is that the compile job doesn't actually know the name of final merged module...
There was a problem hiding this comment.
I'm confused a bit, there is no unmerged or merged module name anymore in the driver. So the comment:
// Recompute the module path based on the module name, because this is effectively passing the output
// of the module merge action which depends on the compile action.
Doesn't really apply because we intend to be passing the output of the -emit-module (EmitModuleJob) action, which does not depend on the compile action.
How are you seeing *-1.swiftmodule names? Taking a look at the EmitModuleJob construction, it seems to just use moduleOutputInfo.output!.outputPath directly itself.
There is no fundamental technical reason to restrict this feature to explict modules only, so this patch adds support for passing the correct -debug-module-path also to implicit builds. rdar://168256846
adrian-prantl
force-pushed
the
eng/PR-168256846
branch
from
4986256 to
b529ffe
Compare
4 participants
There is no fundamental technical reason to restrict this feature to explict modules only, so this patch adds support for passing the correct -debug-module-path also to implicit builds.
rdar://168256846