Skip to content

Conversation

23rd
Copy link

@23rd 23rd commented Nov 27, 2023

The following constant values can be safely computed at compile time, but are currently computing at runtime.

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 27, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #28358 (Double -dbache maximum to 32GB by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@hebasto
Copy link
Member

hebasto commented Nov 27, 2023

Changes to the leveldb subtree must be submitted to upstream, not to this repo.

@23rd
Copy link
Author

23rd commented Nov 27, 2023

Changes to the leveldb subtree must be submitted to upstream, not to this repo.

Thanks. Have updated the commit.

@maflcko
Copy link
Member

maflcko commented Nov 27, 2023

are currently computing at runtime.

Do you have any referenced to support your claim? IIRC all supported compilers treat static const int = ..literal..; as a compile-time constant.

If there is a reason to do the changes, it should ideally be enforced with a linter/tidy check, no?

@23rd
Copy link
Author

23rd commented Nov 27, 2023

@maflcko Oh, I didn't really check asm result of static const int... definitions. Now I see they are the same as constexpr int. Thank you and sorry for my unnecessary PR.

@23rd 23rd closed this Nov 27, 2023
@DrahtBot DrahtBot mentioned this pull request Nov 27, 2023
@bitcoin bitcoin locked and limited conversation to collaborators Nov 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants