Skip to content

Replace runtime C++ version detection with compile-time extraction from argv #349

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

Merged
merged 1 commit into from
Jun 13, 2025

Conversation

anutosh491
Copy link
Collaborator

Description

Fixes #348

Type of change

Please tick all options which are relevant.

  • Bug fix
  • New feature
  • Added/removed dependencies
  • Required documentation updates

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.81%. Comparing base (6dcf09a) to head (9c8c5c1).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #349      +/-   ##
==========================================
+ Coverage   81.78%   81.81%   +0.02%     
==========================================
  Files          20       20              
  Lines         950      957       +7     
  Branches       87       89       +2     
==========================================
+ Hits          777      783       +6     
- Misses        173      174       +1     
Files with missing lines Coverage Δ
src/xinterpreter.cpp 89.94% <100.00%> (-0.17%) ⬇️
Files with missing lines Coverage Δ
src/xinterpreter.cpp 89.94% <100.00%> (-0.17%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@JohanMabille JohanMabille merged commit e0946f7 into compiler-research:main Jun 13, 2025
17 checks passed
@anutosh491 anutosh491 deleted the getstdopt branch June 13, 2025 07:23
@anutosh491
Copy link
Collaborator Author

Thanks for the review !

@vgvassilev
Copy link
Contributor

This is incorrect. Please revert this because that makes sure that we the implementation agrees with the flags.

@JohanMabille
Copy link
Collaborator

Hi Vassil, not sure to get it. Does that mean that even if the kernel passes the standard version to use to CppCompiler, this latter might use a different version for any reason? Meaning the resulting standard used is different from the specified one?

@vgvassilev
Copy link
Contributor

Hi Vassil, not sure to get it. Does that mean that even if the kernel passes the standard version to use to CppCompiler, this latter might use a different version for any reason? Meaning the resulting standard used is different from the specified one?

Hi Johan. We can have -std=c++20 but also for some versions of clang -std=c++2a which should be almost the same thing. If you add /std:c++20 style of msvc the matrix becomes pretty hard to manage. Then, we had a but in the option parsing which made it very difficult to debug what the user thought it passed and what the interpreter actually was running with. That is very problematic when debugging ABI issues with libraries which are compiled with one version but you don't really know which version your system is running on.

That's why we implemented this approach where we really ask the interpreter runtime what version of c++ it really has. It's a bit annoying but saves a lot of effort if something slightly goes wrong (as it did in the past).

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.

Prefer extracting C++ standard version from argv at startup over runtime evaluation
4 participants