pisa-engine/pisa

PISA support for CXX larger than 17

NickNameInvalid opened this issue · 7 comments

Describe the bug
For PISA repo, when I switch the default CXX min_requirement from 17 to 20, I had this error

11

For CXX min requirement from 17 to 23, I got this

image

To Reproduce
Steps to reproduce the behavior:
In pisa/CMakeList.txt, in line 6, change set(CMAKE_CXX_STANDARD 17) to set(CMAKE_CXX_STANDARD 20) or set(CMAKE_CXX_STANDARD 23)

Error message
As shown in Describe the bug section, everything works well when using set(CMAKE_CXX_STANDARD 17)

Environment info
Operating System: Ubuntu 20.0.4
Compiler name: gcc (g++)
Compiler version: 11

Hello,

The bug looks like it's because the version of range-v3 we're using (0.9.1 according to our external directory) is not yet C++-20 ready. I had a quick look at the range-v3 repo and they seem to have fixed these issues in more recent releases.

You could try to upgrade range-v3 locally and see if it builds. But I suspect there may be other similar issues which pervade the entire source tree.

I don't think we necessarily provide support for C++20 - can you just use C++17 or do you have some reason to require 20?

@elshize - any thoughts on this?

Hi Joel, we have a project which required PISA as our C++ repo dependency since we need PISA's wheels of query evaluation, we may need abbreviated function template syntax feature such as auto parameters which is a feature for CXX from 20 (because we would store PISA's returning results and Index structure as member variables in our own data structure) hence it would be really appreciated if PISA could support for CXX 20 and 23

@JMMackenzie I see no reason for not supporting more recent standards. Hopefully it doesn't take too much effort to fix that, but I haven't looked into it yet. I suspect that the problem is namespaces: there might be some ambiguity between range namespace in range-v3 and C++20. If so, then I would imagine that's already fixed in the new versions, so maybe simply upgrading the library will help.

In general, C++20 should be backwards-compatible with C++17. But because the standard library adds functionality, if you're not careful to use namespaces properly, then there should be no problem.

Anyway, I can have a look maybe next week if I have the time, or maybe on Sunday, not sure yet what my schedule is...

@NickNameInvalid Note that C++23 has not yet been published, so any C++23 features your compiler supports are likely to be experimental (for example, see GCC docs: https://gcc.gnu.org/projects/cxx-status.html#cxx23). It's pretty late in the standardization process, so I don't think much, if anything, will change in the final publication, but just be aware of that.

@JMMackenzie BTW I would like to jump on C++20 myself, but I think we probably should stay on 17 still. There is nothing that really holds us back other than being considerate of our users who may not have more recent versions of compilers.

It seems that GCC 9 has a good set of functionality implemented, 10 is almost all, but there are still some things that are in development for the next version (13), like text formatting for example.

Clang is even worse because it has some features completely missing (though none of the major ones).

So at the moment we're stuck with C++17. But I have to say, I am itching to start using concepts. With our template-heavy codebase, this would really improve on the error messages, and maybe even compilation times.

Ok, so in fact it seems like it wasn't necessarily ranges version that was the problem, but we simply had using namespace std in one of our headers! But we may as well upgrade the ranges version, we may have some probelms still because that's a version before C++20 was published.

@NickNameInvalid check out #513 -- it's not finished yet, still need to finish up docs and all, but you could temporarily use that branch if you want to jump on it before I have the time to finish.

@elshize Hi Michal, thanks a lot for your reply and prompt fix, let me update my local PISA branch now!