Skip to content

Make benchmarks compiling in clang-cl #5533

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

AlexGuteniev
Copy link
Contributor

@AlexGuteniev AlexGuteniev commented May 23, 2025

Towards #5479

Most of the time it is warning about deprecated benchmark::DoNotOptimize for constants.

🏗️ Build steps

Build STL as usual:

cmake --preset x64
cmake --build --preset x64

Then add -DCMAKE_CXX_COMPILER=clang-cl to benchmark build:

cmake -B out\bench -S benchmarks -G Ninja -DSTL_BINARY_DIR=out\x64 -DCMAKE_CXX_COMPILER=clang-cl
cmake --build out\bench

@AlexGuteniev AlexGuteniev requested a review from a team as a code owner May 23, 2025 06:06
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews May 23, 2025
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja left a comment

Choose a reason for hiding this comment

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

But I wonder why changes elsewhere are relevant.

Copy link
Contributor Author

@AlexGuteniev AlexGuteniev left a comment

Choose a reason for hiding this comment

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

@frederick-vs-ja the rest are just const that can potentially be optimized, so DoNotOptimize is deprecated

@StephanTLavavej StephanTLavavej self-assigned this May 27, 2025
@StephanTLavavej StephanTLavavej added the test Related to test code label May 27, 2025
@StephanTLavavej
Copy link
Member

Thanks! I cleaned up pre-existing weirdness in priority_queue_push_range.cpp (instead of trying to make Clang happy with it), and more significantly I added these builds to Azure Pipelines. This is very important because otherwise it will immediately bit-rot.

@StephanTLavavej StephanTLavavej removed their assignment May 29, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Ready To Merge in STL Code Reviews May 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Related to test code
Projects
Status: Ready To Merge
Development

Successfully merging this pull request may close these issues.

3 participants