Compile QLever with C++20 #385
Conversation
# Conflicts: # src/util/Serializer/Serializer.h
…th plain char. The simple fix is to remove it - We generally enforce Utf-8 everywhere, so this is fine.
There was a problem hiding this comment.
Thank you very much for this!
There are now no more u8 prefixes, that much I understood. I did not yet understand when you need to add the fs aka fixed_string (several cases) and when you need to add the s suffix (one case) to a string. And in RuntimeInformation.h there still are a few occurrences of u8.
src/engine/RuntimeInformation.h
Outdated
| out << indentStr(indent - 1) << reinterpret_cast<const char*>(u8"├─ ") | ||
| << _descriptor << '\n'; |
There was a problem hiding this comment.
Why do you keep the u8 prefix here, but not in, say, Tokenizer.h ?
There was a problem hiding this comment.
No good reason, I will remove them.
src/parser/Tokenizer.h
Outdated
| */ | ||
| static constexpr auto PnCharsBaseString = fixed_string("A-Za-z"); | ||
|
|
||
| static constexpr auto PnCharsUString = PnCharsBaseString + u8"_"; | ||
| static constexpr auto PnCharsUString = PnCharsBaseString + fs("_"); |
There was a problem hiding this comment.
I read the definition of the fs lambda in the code above, but did not understand why it's needed (as opposed to just calling fixed_string). Can you explain?
There was a problem hiding this comment.
Ok
-
fs("content")is equivalent toctll::fixed_string("content"). It is just shorter, which helps in this local case because it is needed a lot, and then the "actual" contents of the string become more visible. -
There is an overload for
operator+forfixed_stringand compile time knownchar[]constants. Thus we do not have to writefs(...orfixed_stringfor every little string when concatenating, but only for one of them. In the places where I changedu8"content"to 'fs("content")I could also just have gone"content"`. Do you want me to make this consistent?
There was a problem hiding this comment.
Thanks for the explanation.
@1: OK, but then maybe the comment, where the fs lambda is defined could be clarified. Right now it says "Is needed for some magical reasons".
@2: What about expressions like PnCharsUString + fs("\\-0-9") or cls(PnCharsBaseString) + fs("(\\.") + ..., is it needed there? Or is it only needed for the first string in a sum?
| @@ -141,7 +141,7 @@ bool TurtleParser<T>::predicateSpecialA() { | |||
| _tok.skipWhitespaceAndComments(); | |||
| if (auto [success, word] = _tok.template getNextToken<TokId::A>(); success) { | |||
| (void)word; | |||
| _activePredicate = u8"<http://www.w3.org/1999/02/22-rdf-syntax-ns#type>"; | |||
| _activePredicate = "<http://www.w3.org/1999/02/22-rdf-syntax-ns#type>"s; | |||
There was a problem hiding this comment.
Why is the s needed here and not in other places?
There was a problem hiding this comment.
It is not needed. The s suffix makes a std::string constant, whereas the "" is just a const char* which can also be implicitly converted to a std::string. If it confuses you, I can remove it again.
| constexpr ctre::fixed_string r1(R"(\xc3([\x80-\x96]|[\x98-\xb6]|[\xB8-\xBF]))"); | ||
| constexpr ctre::fixed_string r2(R"([\xc4-\xcb][\x80-\xBF])"); | ||
| constexpr ctre::fixed_string r3( | ||
| constexpr ctll::fixed_string r1(R"(\xc3([\x80-\x96]|[\x98-\xb6]|[\xB8-\xBF]))"); |
There was a problem hiding this comment.
Why ctll:: instead of ctre:: now?
There was a problem hiding this comment.
Don't ask me. They stopped compiling with the ctre:: namespace, but it's the same type.
This PR sets the Dockerfile and CMakeLists.txt to use C++20. It is the future and will allow us to write much more readable code, especially when it comes to template-metaprogramming.
This PR applies also several small fixes to make QLever actually compile with C++20, most notably:
u8-Strings like u8"äpfel" now have type
char8_t[]which is not compatible withchar. I have removed this prefix, because it is not necessary (we enforce Utf-8 everywhere anyway).Small fixes for the stxxl-related submodules (I have contacted the team, to find out, whether stxxl is still maintained or whether we will have to maintain our own fork in the future).