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

Remove the unnecessary NoDestructor in unstoppable_domains::GetRecordKeys #42453

Closed
cdesouza-chromium opened this issue Nov 22, 2024 · 0 comments · Fixed by brave/brave-core#26698

Comments

@cdesouza-chromium
Copy link
Contributor

Description

That static should be a simple constexpr constant.

@cdesouza-chromium cdesouza-chromium self-assigned this Nov 22, 2024
cdesouza-chromium added a commit to brave/brave-core that referenced this issue Nov 22, 2024
This function has been returning a vector of strings, to be used by
`GetMany`, and therefore a `NoDestructor` instance is needed. However,
the only reason why this has to be a vector of strings is because in the
end `EncodeStringArray` requires a vector of strings.

This PR changes `EncodeStringArray` to take spans of strings and of
string views. This will allows to convert the keys into a `constexpr`
value, and avoid the use of `NoDestructor`.

In a follow up PR, additional callsites of `EncodeStringArray` may be
migrated to the `string_view` variant.

This is a follow up of these PRs:
#26683
#26688

Resolves brave/brave-browser#42453
cdesouza-chromium added a commit to brave/brave-core that referenced this issue Nov 22, 2024
This function has been returning a vector of strings, to be used by
`GetMany`, and therefore a `NoDestructor` instance is needed. However,
the only reason why this has to be a vector of strings is because in the
end `EncodeStringArray` requires a vector of strings.

This PR changes `EncodeStringArray` to take spans of strings and of
string views. This will allows to convert the keys into a `constexpr`
value, and avoid the use of `NoDestructor`.

In a follow up PR, additional callsites of `EncodeStringArray` may be
migrated to the `string_view` variant.

This is a follow up of these PRs:
#26683
#26688

Resolves brave/brave-browser#42453
@brave-builds brave-builds added feature/web3/wallet Integrating Ethereum+ wallet support feature/web3/wallet/core labels Nov 23, 2024
@brave-builds brave-builds added this to the 1.75.x - Nightly milestone Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants