Skip to content

Dont check message on tests since this fails on newer libc++ #7656

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 1 commit into
base: main
Choose a base branch
from

Conversation

pfultz2
Copy link
Contributor

@pfultz2 pfultz2 commented Jul 8, 2025

No description provided.

@firewave
Copy link
Collaborator

firewave commented Jul 9, 2025

Which libc++ on which system is failing? I am using LLVM 20 and 21 locally and so far the tests passed.

The problem with not checking the actual messages is that it might throw the same type but for the wrong reasons.

@pfultz2
Copy link
Contributor Author

pfultz2 commented Jul 9, 2025

Which libc++ on which system is failing? I am using LLVM 20 and 21 locally and so far the tests passed.

I am using apple clang version 16, but the libc++ version is 18.1 so this fails.

The problem with not checking the actual messages is that it might throw the same type but for the wrong reasons.

Then why doesnt it use a different type? Checking messages seems very fragile.

Also, why dont we use std::stod or std::strtod instead for these functions?

@firewave
Copy link
Collaborator

firewave commented Jul 9, 2025

I am using apple clang version 16, but the libc++ version is 18.1 so this fails.

I was afraid that there might be a discrepancy between those. I tested the CI with the various available versions to check the proper value. But is is weird that it depends on the compiler version and not the standard library version. So that probably needs to add a check for __clang_major__.

Then why doesnt it use a different type? Checking messages seems very fragile.

Because we would need that type for the tests only. And I do not like to have test-only code in the application code. And do you really want to pile on more on the mess which is the exception handling within the analysis?

Also, why dont we use std::stod or std::strtod instead for these functions?

These different messages are coming from exactly those functions - which we only call with libc++ because std::istringstream is broken in it. Otherwise there would be no such hacks at all.

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.

2 participants