cloudius-systems / osv

OSv, a new operating system for the cloud.
osv.io
Other
4.12k stars 605 forks source link

kernel: upgrade to C++ 17 #1289

Closed wkozaczuk closed 3 months ago

wkozaczuk commented 11 months ago

This patch makes necessary changes to the relevant source files to make it possible to build kernel at C++ 17 level. This will allow us to take advantage of new features available in C++ 17 standard.

The following changes are part of this PR:

An example of how to build with C++ 17:

./scripts/build image=httpserver-api fs=rofs.fg -j$(nproc) conf_cxx_level=gnu++17

Finally, this PR does not change the default C++ level which remains at 11, until we permanently decide to change in the future.

nyh commented 11 months ago

I'm curious, what is the motivation of this change. Which C++17 features are you planning to use? But I admit it's kind of silly to stick to the 12 year old C++11 standard, so I won't oppose this change.

wkozaczuk commented 11 months ago

@nyh I have some ideas on how to tackle #853 and I needed a way to re-purpose existing waitlist objects and instead of deleting them, keep them (your idea to comment out erase) and "re-key" unused entries by doing extract and insert (see https://www.fluentcpp.com/2020/05/01/how-to-change-a-key-in-a-map-or-set-in-cpp/). This is to minimize allocations.

wkozaczuk commented 11 months ago

I have relied on the compiler to tell me what is wrong and then verify things still work by using unit tests. But based on your experience are there potentially things that may get broken by this change and we would not even know about it? (see 'Changes to evaluation order' or 'Guaranteed copy elision' in https://developers.redhat.com/articles/2021/08/06/porting-your-code-c17-gcc-11)

wkozaczuk commented 3 months ago

@nyh I have updated this PR. The changes include fixes to core/callstack.cc and adding new build config option conf_cxx_level. So the kernel and all relevant modules get built with non-c++_11 only if explicitly specified.