Fix -Wdeprecated-this-capture warning #17882
Closed
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
acozzette
added
the
🅰️ safe for tests
acozzette
approved these changes
Aug 28, 2024
github-actions
bot
removed
the
🅰️ safe for tests
|
This seems to cause some compiler errors like this one: |
`this` needs to be explicitly captured to avoid the warning.
🤦 thanks. fixed |
tonyliaoss
added
the
🅰️ safe for tests
github-actions
bot
removed
the
🅰️ safe for tests
copybara-service bot
pushed a commit
that referenced
this pull request
Oct 10, 2024
See actions/runner-images#10721. #test-continuous COPYBARA_INTEGRATE_REVIEW=#17882 from dotnwat:patch-1 348ce1b FUTURE_COPYBARA_INTEGRATE_REVIEW=#17882 from dotnwat:patch-1 348ce1b PiperOrigin-RevId: 683309141
copybara-service bot
pushed a commit
that referenced
this pull request
Oct 10, 2024
…string.
As an optimization, we maintain a default instance of a string in memory, and
messages with uninitialized fields will be constructed with a pointer to this
default string object. The destructor should clear the field only when it is
"set" to a nondefault object.
We additionally maintain the invariant that, if hasbit exists for the field,
that the hasbit is only set when the string is nondefault. In other words,
hasbit is set iff string field points to a "real" string object instead of the
default one.
This invariant allows us to deallocate the string directly if hasbit is set,
without an additional branch to check that the string is nondefault.
If `ClearNonDefaultToEmpty()` is ever called on a default string...
- it's ok in the sense that you're just overwriting an object in memory with a
bunch of zeros.
- but it's quite bad for a number of reasons:
1. It can confuse TSAN.
2. It blocks us from moving the default instance of the string into the
`.data` section. Having the default instance of the string live in
read-only memory would be beneficial for memory safety. It would play well
with hugepages. It would also eliminate some runtime cost of instantiating
the default instance of the string.
This change adds a debug-fail so that users don't call this function "unsafely".
COPYBARA_INTEGRATE_REVIEW=#17882 from dotnwat:patch-1 348ce1b
FUTURE_COPYBARA_INTEGRATE_REVIEW=#17882 from dotnwat:patch-1 348ce1b
PiperOrigin-RevId: 683766870
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
thisneeds to be explicitly captured to avoid the warning.