Exceptions are thrown in a way not supported by C++17

[Drvanon]

In the processes of trying to reinstall I encountered the error at the bottom of this file. This is a libtensorlight problem, but I could not add the issue at that repository. I will try to compile using C++11, but I think this error is worth wile investigating from the simple perspective of maintenance.

In file included from /home/robin/.local/include/libtensor/symmetry/product_table_container.h:8,
                 from libadcc/MoSpaces/setup_point_group_table.hh:24,
                 from libadcc/MoSpaces.cc:22:
/home/robin/.local/include/libtensor/symmetry/product_table_i.h:91:24: error: ISO C++17 does not allow dynamic exception specifications
   91 |     void check() const throw(bad_symmetry);
      |                        ^~~~~
In file included from libadcc/MoSpaces/setup_point_group_table.hh:24,
                 from libadcc/MoSpaces.cc:22:
/home/robin/.local/include/libtensor/symmetry/product_table_container.h:54:41: error: ISO C++17 does not allow dynamic exception specifications
   54 |     void add(const product_table_i &pt) throw(bad_parameter);
      |                                         ^~~~~
/home/robin/.local/include/libtensor/symmetry/product_table_container.h:62:39: error: ISO C++17 does not allow dynamic exception specifications
   62 |     void erase(const std::string &id) throw(bad_parameter, generic_exception);
      |                                       ^~~~~
/home/robin/.local/include/libtensor/symmetry/product_table_container.h:72:36: error: ISO C++17 does not allow dynamic exception specifications
   72 |             const std::string &id) throw(bad_parameter, exception);
      |                                    ^~~~~
/home/robin/.local/include/libtensor/symmetry/product_table_container.h:85:36: error: ISO C++17 does not allow dynamic exception specifications
   85 |             const std::string &id) throw (bad_parameter, exception);
      |                                    ^~~~~
/home/robin/.local/include/libtensor/symmetry/product_table_container.h:89:55: error: ISO C++17 does not allow dynamic exception specifications
   89 |     const PTT &req_const_table(const std::string &id) throw (generic_exception);
      |              
/home/robin/.local/include/libtensor/symmetry/product_table_container.h:96:43: error: ISO C++17 does not allow dynamic exception specifications
   96 |     void ret_table(const std::string &id) throw(bad_parameter);
      |                                           ^~~~~
/home/robin/.local/include/libtensor/symmetry/product_table_container.h:111:32: error: ISO C++17 does not allow dynamic exception specifications
  111 |         const std::string &id) throw (generic_exception) {
      |                                ^~~~~
error: command '/usr/bin/gcc' failed with exit code 1

[Drvanon]

For posterity: I solved this by setting the compiler to use C++11 by running export CFLAGS='-std=c++11' in the shell that I run ./setup.py install.

[mfherbst]

That's annoying. Can you add an issue at https://github.com/adc-connect/libtensor? Or even better: Open a PR over there to fix it. Roughly all it takes is to remove the exception declarations explcitly.

[maxscheurer]

I think Issues are disabled for our libtensor fork... PR would be much appreciated! 😄 👍

[mfherbst]

Ah true ... maybe we should enable that.

[Drvanon]

I am trying to do the PR, and I think the solution is just to do a general replacement of throw (generic_exception) to throw (). If I understand the comments on this correctly. Another option would be to use noexcept(false), but I admit to not understanding C++17 exceptions well enough to be authoritative on this.

[mfherbst]

Basically C++17 deprecates throw declarations, so just remove the throw( ... ) attributes completely.

[Drvanon]

Sweet, I will than throw the following regex over the code: throw \((.*?)\). My plan is to replace all that with an empty string. Side note: turning on C++17 compliance on your project in the CMakeLists.txt file generators a frightening amount of warnings, most of which can be removed by taking away the register keyword from for loops. I don't know if you would like me to do that too, but if so that is no problem.

[maxscheurer]

Probably we should stick to C++11/C++14 as CMAKE_CXX_STANDARD for the time being, @mfherbst? I just checked other packages, Psi4 still at C++14, so probably we should not bump this... could also make conda package compatibility more difficult than needed.

[mfherbst]

register is deprecated too and can be removed. I don't mind in hard-enforcing C++14 for adcc. But the work of @Drvanon does not require C++17. Rather it allows C++17 (and leads to a requiremend for C++11, which is fair).

[maxscheurer]

But the work of @Drvanon does not require C++17.

I know 😄 But he was asking whether to do this or not... Long story short: leave all C++ standards as is 😆

[mfherbst]

ah yes, definitely :smile: ... sorry 'bout the confusion.

[Drvanon]

Sweet, I will than throw the following regex over the code: throw \((.*?)\). My plan is to replace all that with an empty string.

Hmmm, just went through the changes. This does remove everywhere were throw is used (naturally), but that also implies not throwing any error, ever. Is this the intention? edit: Because there is only 10 lines, I will just look at them by hand.

[mfherbst]

No better not... What I had in mind was to only remove the throw declaration in the function definition and not the throw statements. In that sense your regex might be too wide. I have not checked. Just fix that by only committing the appropriate lines (git add -p). I.e. remove

int test() throw (myexcept) {
    ...
}

but not

int test() {
    throw (myexcept) 
}

[Drvanon]

Since I am completely unfamiliar with the throw functionality in c++, is the const part of the throw in the following piece:

template<typename ListT>
    void populate(ListT &list, const dimensions<k_ordera> &dima,
        const dimensions<k_orderb> &dimb,
        const dimensions<k_orderc> &dimc) const throw(exception);

[maxscheurer]

is the const part of the throw in the following piece:

No. I'm certain that const has nothing to do with throw.

[mfherbst]

discussion continues in https://github.com/adc-connect/libtensor/pull/12