Increase C++ version from 17 to 20

[soheilshahrouz]

To see if CI passes.

[AmirhosseinPoolad]

There's been a breaking change in C++20 regarding templates and constructors, a lot of the warnings and errors are because of that.

https://godbolt.org/z/7n6saxzee
https://stackoverflow.com/questions/63513984/can-class-template-constructors-have-a-redundant-template-parameter-list-in-c2

[AlexandreSinger]

@AmirhosseinPoolad That is correct, I recall seeing warnings of that when I was upgrading to newer versions of GCC. Those actually should be fixed anyways since they are strange. The issue comes from:

template<T>
class MyClass {
  MyClass<T>(...); // constructor
};

The issue with the code above is that it looks like you are specifying a specific constructor for a specific type. But that type is the template argument. I think this causes duplicate constructors in the compiler which may increase compile time (or at worst cause the constructor to be all wonky). This error in c++20 may be actually fixing some of these issues.

The code above is trivially fixed:

template<T>
class MyClass {
  MyClass(...); // constructor
};

I think I already did this for the vtr ndmatrix class.

[AmirhosseinPoolad]

Talked to Soheil about it and there's also a whole lot of errors/warnings because C++20 requires more functions to be declared noexcept and we have -Wnoexcept on. Also trivially fixable (AFAIK) but it's a lot of busy work.

[AlexandreSinger]

@soheilshahrouz FYI, I do not think this is an error. I think this is a warning. But for the regression test and treat all warnings as errors to keep the code warning clean.

We explicitly turn on these warnings in the CMake file here:

vtr-verilog-to-routing/CMakeLists.txt

Lines 173 to 175 in b8c1ef1

	"-Wdisabled-optimization" #Warn when optimizations are skipped (usually due to large/complex code)
	"-Wnoexcept" #Warn when functions should be noexcept (i.e. compiler know it doesn't throw)
	"-Woverloaded-virtual" #Warn when a function declaration overrides a virtual method

It could be possible that our code is causing this issue, but based on what I found online it may be the STL that has some issues:
https://stackoverflow.com/a/59381014

I think we can find ways to add noexcept to some classes (for example, strong IDs seem to be the main culprit!).

Worst case scenario, we can turn this warning off; but I do not like that idea.

[soheilshahrouz]

Compilation with clang 15 fails. It is probably related to this and this.

Is it ok to remove clang 15 from CI tests?

[AlexandreSinger]

Compilation with clang 15 fails. It is probably related to this and this.

Is it ok to remove clang 15 from CI tests?

I agree with removing Clang 15 for the reason that Soheil gives. The culture behind Clang releases is slightly different from GCC, where older versions do not get as much long term support as GCC in my opinion. Based on the Clang C++ standard support page (here), Clang 16 and 17 saw a lot of the c++-20 support. Based on the GCC c++ standard support page (here), GCC-11 should support basically all of c++-20 features.

To replace clang 15, we may be want to add Clang 19 to the CI (which is the most recent version of Clang I could find; but I am not sure if the GitHub runners support this version of Clang or not yet). However, that should be done in a different PR.

@vaughnbetz What do you think? If you agree, I can make Clang 15 no longer be a required test and we can turn off that test in this PR.

[AlexandreSinger]

These changes all LGTM. Even without changing c++ version, all of these changes are good to have and may make the code slightly faster (will allow the compiler to make things a bit faster).

[vaughnbetz]

I'm OK with dropping clang 15 to enable C++ 20

[AlexandreSinger]

I have removed Clang-15 from the branch protection rules on the Master branch (it is no longer required). @soheilshahrouz feel free to remove the Clang-15 test from the CI tests.

[vaughnbetz]

LGTM