[Explicit Module Builds] Add a setting to allow creating a reproducer for errors. #821
+22
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… for errors. The idea is the same as Clang flag `-gen-reproducer=error` but for explicit module builds. The name of the setting is a subject to change depending on how it ends up being used. Not all module-related issues are crashes, so it is useful to allow creating and sharing reproducers for errors. rdar://158780565
vsapsai
requested review from
aciidgh,
jakepetroules,
mhrawdon,
neonichu and
owenv
as code owners
vsapsai
commented
Sep 26, 2025
Comment on lines
+322
to
+324
| let shouldGenerateReproducer = (lastResult == .failed) && | ||
| (explicitModulesPayload.shouldGenerateReproducerForErrors || | ||
| (outputDelegate.result?.isCrashed ?? false)) |
There was a problem hiding this comment.
I've spent some time trying to improve this code, so let me share a few options I've discarded. If you prefer one of the mentioned options, let me know
let shouldGenerateReproducer: Bool
if lastResult == .failed {
if explicitModulesPayload.shouldGenerateReproducerForErrors {
shouldGenerateReproducer = true
} else if case .exit(exitStatus: .uncaughtSignal, metrics: _) = outputDelegate.result {
// In case of crash.
shouldGenerateReproducer = true
} else {
shouldGenerateReproducer = false
}
} else {
shouldGenerateReproducer = false
}Slight variation with if-expression
let shouldGenerateReproducer = if lastResult == .failed {
if explicitModulesPayload.shouldGenerateReproducerForErrors {
true
} else if case .exit(exitStatus: .uncaughtSignal, metrics: _) = outputDelegate.result {
// In case of crash.
true
} else {
false
}
} else {
false
}Try to add case
let shouldGenerateReproducer = if lastResult == .failed {
switch (explicitModulesPayload.shouldGenerateReproducerForErrors, outputDelegate.result) {
case (true, _): true
case (_, .some(.exit(exitStatus: .uncaughtSignal, metrics: _))): true
default: false
}
} else {
false
}
There was a problem hiding this comment.
No strong opinion from me, seems fine as it is.
jakepetroules
approved these changes
Sep 26, 2025
|
@swift-ci test |
1 similar comment
|
@swift-ci test |
|
As far as I can tell all the failed tests are in ClangCompilationCachingTests.swift that is unrelated to my change. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The idea is the same as Clang flag
-gen-reproducer=errorbut for explicit module builds. The name of the setting is a subject to change depending on how it ends up being used.Not all module-related issues are crashes, so it is useful to allow creating and sharing reproducers for errors.
rdar://158780565