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.

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

Disable exceptions on ICPC #3621

Merged
merged 1 commit into from
Jul 30, 2022

Conversation

falbrechtskirchinger
Copy link
Contributor

We currently don't disable exceptions on ICPC for test-disabled_exceptions. Let's see what happens if we do.

Also simplifies the CMake code a lot.

@falbrechtskirchinger
Copy link
Contributor Author

CMake Error at cmake/test.cmake:136 (target_link_libraries):
Error evaluating generator expression:

$<CXX_COMPILER_ID:AppleClang,Clang,GNU,Intel,IntelLLVM>

$<CXX_COMPILER_ID> expression requires one or zero parameters.

Apparently a CMake 3.15 feature. :-(

$<CXX_COMPILER_ID:compiler_ids>
where compiler_ids is a comma-separated list. 1 if CMake's compiler id of the CXX compiler matches any one of the entries in compiler_ids, otherwise 0.

@nlohmann
Copy link
Owner

Doesn’t this run in the CI container? There we have control over the CMake version.

@falbrechtskirchinger
Copy link
Contributor Author

falbrechtskirchinger commented Jul 29, 2022

The CMake flag test uses 3.13. That one fails.

3.13 was released around Nov 2018. Bumping to 3.15 might be a bit too soon for some distros. 🤷‍♂️

@nlohmann
Copy link
Owner

The CMake flag test uses 3.13. That one fails.

3.13 was released around Nov 2018. Bumping to 3.15 might be a bit too soon for some distros. 🤷‍♂️

Yes, the 3.13 is for externals to execute the unit tests. Not sure if the specific "Intel without exceptions" should be seen as a test someone other than a maintainer should execute. I guess this goes in a similar direction as #3623 - we need to better separate maintainer targets from the rest.

@falbrechtskirchinger
Copy link
Contributor Author

Unfortunately, I can't separate them. Not if I want to replace the if() construct with a single generator expression.

Basically, I can make it work, but I can't also clean up the CMake code. Oh well …

@nlohmann
Copy link
Owner

Unfortunately, I can't separate them. Not if I want to replace the if() construct with a single generator expression.

Basically, I can make it work, but I can't also clean up the CMake code. Oh well …

Please don't invest more work to this as necessary...

@falbrechtskirchinger
Copy link
Contributor Author

I won't. I'll revert and just add "Intel" and "IntelLLVM" to the if().

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 824f629 on falbrechtskirchinger:intel-no-exceptions into d1d79b9 on nlohmann:develop.

@falbrechtskirchinger falbrechtskirchinger marked this pull request as ready for review July 29, 2022 15:54
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@nlohmann nlohmann self-assigned this Jul 30, 2022
@nlohmann nlohmann added this to the Release 3.11.0 milestone Jul 30, 2022
@nlohmann nlohmann merged commit a2578d1 into nlohmann:develop Jul 30, 2022
@falbrechtskirchinger falbrechtskirchinger deleted the intel-no-exceptions branch July 30, 2022 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants