Skip to content

Fix compilation with C++17 and later - #281

Merged
balay merged 2 commits into
elemental:masterfrom
prj-:jolivet/fix-cpp17
Oct 13, 2026
Merged

balay merged 2 commits into
elemental:masterfrom
prj-:jolivet/fix-cpp17

Conversation

@prj-

@prj- prj- commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@prj-

prj- commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

@andreasnoack, very trivial PR, would you mind reviewing/integrating it? Thanks in advance.

@balay

balay commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

https://github.com/elemental/Elemental/actions/runs/38260879078/job/114897739085

@prj-

I'm rerunning the failed job. But the following is the likely CI fix:

--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -33,6 +33,7 @@
       CC: ${{ matrix.cc }}
       CXX: ${{ matrix.cxx }}
       FC: ${{ matrix.f77 }}
+      UCX_TLS: tcp,self,sm

     steps:
       - uses: actions/checkout@v4

Also claude suggests:

--- a/include/El/core/environment/decl.hpp
+++ b/include/El/core/environment/decl.hpp
@@ -37,8 +37,7 @@
 using std::exception;
 inline bool has_uncaught_exception() noexcept
 {
-#if __cplusplus >= 201703L || \
-    (defined(_MSVC_LANG) && _MSVC_LANG >= 201703L)
+#ifdef __cpp_lib_uncaught_exceptions
     return std::uncaught_exceptions() > 0;
 #else
     return std::uncaught_exception();

@balay

balay commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

I'm rerunning the failed job

so that worked!

@prj-

prj- commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

I didn't know you had write access here. That simplifies everything.

@balay

balay commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

create a PR with some additional CI changes at #282

@prj-

prj- commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Will apply your suggestions after the other PR is merged.

@prj-
prj- force-pushed the jolivet/fix-cpp17 branch from 9b1f3eb to 96fc476 Compare October 13, 2026 12:36
@prj-

prj- commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

I've applied your suggestion for include/El/core/environment/decl.hpp, and left the one for .github/workflows/ci.yml since it should leave in your other PR.

@balay

balay commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

calude is suggesting one more fix, I think good to apply:

include/El/core/environment/decl.hpp:40 tests __cpp_lib_uncaught_exceptions, but <exception> (and <version>) is never included — core.hpp:16-32 has <stdexcept> and friends, not <exception>.

  It works today only because libstdc++ pulls <exception> in transitively 
diff --git a/include/El/core.hpp b/include/El/core.hpp
index 0f89acaed..fde84a01e 100644
--- a/include/El/core.hpp
+++ b/include/El/core.hpp
@@ -20,6 +20,7 @@
 #include <cstdlib>
 #include <cstring>
 #include <ctime>
+#include <exception>
 #include <fstream>
 #include <functional>
 #include <iostream>

@balay

balay commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Since this is simpler MR (than #282) - I'll go ahead and merge this in (after the above fix, and a successful CI run).

@balay

balay commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Hm - opencode is suggesting:

noexcept → EL_NO_EXCEPT — matches the convention already used elsewhere in the same file
diff --git a/include/El/core/environment/decl.hpp b/include/El/core/environment/decl.hpp
index c236307e0..e87c46441 100644
--- a/include/El/core/environment/decl.hpp
+++ b/include/El/core/environment/decl.hpp
@@ -9,6 +9,8 @@
 #ifndef EL_ENVIRONMENT_DECL_HPP
 #define EL_ENVIRONMENT_DECL_HPP
 
+#include <exception>
+
 namespace El {
 
 using std::size_t;
@@ -35,7 +37,7 @@ using std::ostream;
 using std::ostringstream;
 
 using std::exception;
-inline bool has_uncaught_exception() noexcept
+inline bool has_uncaught_exception() EL_NO_EXCEPT
 {
 #ifdef __cpp_lib_uncaught_exceptions
     return std::uncaught_exceptions() > 0;

@prj-

prj- commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

I don't get why we should #include <exception>, there is not a single #include in this file.

@prj-
prj- force-pushed the jolivet/fix-cpp17 branch from 96fc476 to f386f93 Compare October 13, 2026 19:25
@prj-
prj- force-pushed the jolivet/fix-cpp17 branch from f386f93 to c87ec25 Compare October 13, 2026 19:26
@prj-

prj- commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Applied Claude suggestion and the OpenCode second suggestion.

@balay
balay merged commit d1c93f6 into elemental:master Oct 13, 2026
7 of 9 checks passed
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