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.

Sign up for GitHub

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

Jump to bottom

Clang 18 on MacOS tests fail due to uncaught (but intentional) exceptions #1490

Open
KrisThielemans opened this issue Aug 19, 2024 · 2 comments · May be fixed by #1496
Open

Clang 18 on MacOS tests fail due to uncaught (but intentional) exceptions #1490

KrisThielemans opened this issue Aug 19, 2024 · 2 comments · May be fixed by #1496
Milestone
v6.3

Comments

@KrisThielemans
Copy link
Collaborator

We have a few tests that check if errors are thrown, e.g.

STIR/src/test/test_DateTime.cxx

Lines 105 to 114 in 209cbb6

std::cerr << "\nThe next test should throw an error\n";
try
{
Interfile_datetime_to_secs_since_Unix_epoch(DateTimeStrings("1971:02:2", "23:03:01"));
check(false, "test ill-formed date");
}
catch (...)
{
std::cerr << "Test was ok\n";
}

However, with clang18 on MacOS these tests all fail
https://github.com/UCL/STIR/actions/runs/10450627407/job/28935385772?pr=1489#step:12:751

Testing DICOM DateTime to epoch functionality

The next test should throw an error

ERROR: Time_Zone info '+020' does not fit DICOM standard
libc++abi: terminating due to uncaught exception of type std::runtime_error: 
ERROR: Time_Zone info '+020' does not fit DICOM standard

This is pretty weird, at catch (...) should catch all exceptions AFAIK, so this seems like a clang bug to me.

However, we know what we're throwing

STIR/src/buildblock/error.cxx

Line 62 in 209cbb6

throw std::runtime_error(msg);

So we could just as well (or even better) catch std::runtime_error.
@KrisThielemans KrisThielemans added this to the v6.3 milestone Aug 19, 2024
This was referenced Aug 19, 2024
Merged
Draft
KrisThielemans added a commit to KrisThielemans/STIR-1 that referenced this issue Aug 25, 2024
@KrisThielemans
change tests with intentional errors to catch std::runtime_error
e1e300f 
Some tests check if errors are thrown. We caught any exception with
`catch (...)`. but that fails on OSX  Clang 18. Now we catch
`std::runtime_error`, which is more accurate anyway.

Fixes UCL#1490
KrisThielemans added a commit to KrisThielemans/STIR-1 that referenced this issue Aug 25, 2024
@KrisThielemans
change tests with intentional errors to catch std::runtime_error
e27c966 
Some tests check if errors are thrown. We caught any exception with
`catch (...)`. but that fails on OSX  Clang 18. Now we catch
`std::runtime_error`, which is more accurate anyway.

Fixes UCL#1490
@KrisThielemans
Copy link
Collaborator Author

Catching std::runtime_error& as in
https://github.com/UCL/STIR/pull/1496/files#diff-8edd730b56a9f4c3e9faa36d393762b837fd91893dda5e4db881a97f3a0c5cf4
does not help.
https://github.com/UCL/STIR/actions/runs/10545168824/job/29215018326#step:12:119

You should now see an error about a wrong setting for alpha

ERROR: Alpha parameter for ramp has to be between 0 and 1 but is -1
libc++abi: terminating due to uncaught exception of type std::runtime_error: 
ERROR: Alpha parameter for ramp has to be between 0 and 1 but is -1

@KrisThielemans KrisThielemans mentioned this issue Feb 20, 2025
Merged
2 tasks
@KrisThielemans
Copy link
Collaborator Author

@ashgillman found possibly related llvm/llvm-project#94292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v6.3
Development

Successfully merging a pull request may close this issue.

fix OSX clang problems KrisThielemans/STIR-1
1 participant
@KrisThielemans