Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tighten up handling of debug mode #1138

Merged
merged 1 commit into from
Nov 9, 2023
Merged

tighten up handling of debug mode #1138

merged 1 commit into from
Nov 9, 2023

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Oct 28, 2023

This does a few things @jayz22 and I discussed today:

  1. Eliminates the second callback and the value-passing on the shadow and debug mode callbacks in favor of mutating a value outside the closure when observation is required. This is not necessarily better, just a little more awkward / less likely to be done accidentally without thinking about it.
  2. Removes all convenience accessors for the debug flag, again just to make it harder to accidentally observe.
  3. Feature-gates DebugInfo on testutils, so it's never live in production. Note that DebugInfo is not the diagnostic event buffer, it's a snapshot of the buffer along with a (native) backtrace attached to errors that get printed to console in a panic catch, it only makes sense for testing and by eliminating it here we can actually compile out all support for backtraces (including the dwarf-reading crate) from production. Which is just one less pile of software to worry about shipping (this is also good because backtrace is going to be stable in the rust stdlib soon).

@graydon graydon requested review from jayz22 and removed request for sisuresh October 28, 2023 08:11
Copy link
Contributor

@jayz22 jayz22 left a comment

Choose a reason for hiding this comment

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

This is great! Really nice to see the diagnostic workflows getting safer.

@graydon graydon requested a review from dmkozh November 1, 2023 18:23
@graydon graydon added this pull request to the merge queue Nov 4, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Nov 4, 2023
@graydon graydon added this pull request to the merge queue Nov 9, 2023
Merged via the queue into main with commit 2d6456c Nov 9, 2023
10 checks passed
@graydon graydon deleted the tighten-debug branch November 9, 2023 22:01
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