Skip to content

Conversation

@adrian-prantl
Copy link

These diagnostics are useful when debugging LLDB and project configurations, but in a live debug session they mostly get in the way. Especially when using explicit modules mismatches in warning options caused by implicitly triggered imports are not really actionable fo users outside of clearing their module cache and trying again.

rdar://130284825

@adrian-prantl
Copy link
Author

@swift-ci test

@kastiglione
Copy link

kastiglione commented Aug 5, 2024

In our conversation last week, I thought it was said that validation would be on by default. If the default is to be disabled, should it only be disabled for explicit modules?

Another thought: the validation checks warning/diagnostic flags, which we know we want to disable, but also checks language options (and target options, etc), which I'm not sure we want to disable. Should we add logic into the validation so that we can take the safer action of disabling only warnings/diagnostics validation?

Choose a reason for hiding this comment

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

This is from memory, but I believe pch validation gates the module relocation checks. In other words, disabling pch validation should make it unnecessary to explicitly disable module-check-relocated.

@JDevlieghere
Copy link

Would it make sense to keep "on" in debug/assert builds, an defaulting to "off" for release builds?

These diagnostics are useful when debugging LLDB and project
configurations, but in a live debug session they mostly get in the
way. Especially when using explicit modules mismatches in warning
options caused by implicitly triggered imports are not really
actionable fo users outside of clearing their module cache and trying
again.

rdar://130284825
@adrian-prantl
Copy link
Author

I think I addressed everyone's review comments. It's now set to auto by default, which means on iff explicit modules are on, or if we are in an asserts build. Because of that the test got weaker.

@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl
Copy link
Author

@swift-ci test windows

Copy link

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

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

LGTM. I don't mind the AutoBool but I wonder if we don't have "better" support for LLDB's LazyBool.

@adrian-prantl adrian-prantl merged commit 31f0cdc into swiftlang:swift/release/6.0 Aug 6, 2024
@adrian-prantl
Copy link
Author

LGTM. I don't mind the AutoBool but I wonder if we don't have "better" support for LLDB's LazyBool.

Great question, I basically copied & pasted this from another setting assuming that that's the way.

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