openzim / zim-tools

Various ZIM command line tools
https://download.openzim.org/release/zim-tools/
GNU General Public License v3.0
133 stars 35 forks source link

Use of C++20 features/extensions #432

Open ThisIsFineTM opened 1 month ago

ThisIsFineTM commented 1 month ago

The src/zimwriterfs/zimcreatorfs.cpp file uses designated initializers, which is a c++20 feature. It currently works by relying on compiler c++20-extensions with the c++ standard set to c++17 in meson.build.

Are there any dependency blockers for just changing the c++ standard version to c++20? If there aren't any blockers I can put in a PR.

kelson42 commented 1 month ago

We want to keep things compiling on older compilers. The strategy is to allow c++ 14 features but not higher.

ThisIsFineTM commented 1 month ago

It would be helpful if we could figure out which minimum compiler version we specifically want to target. The codebase already uses some >= C++17 features, so those sections would need a downgrade to comply with the intended target of C++14. Is the Repology page accurate?

kelson42 commented 1 month ago

We have to compile on cpp14 compilers, therefore we should not use moderner breaking forms.

Do you have some kind of detailed report showing that the code is not cpp14 compliant anymore?

ThisIsFineTM commented 1 month ago

Hi Kelson,

Thank you for the quick reply. Here are the ones that stood out to me.

https://github.com/openzim/zim-tools/blob/a65cb01f63d391f6684e823f5d6c3dcef3ce7fda/meson.build#L4

4: default_options : ['c_std=c11', 'cpp_std=c++17', 'werror=true', 'warning_level=1'])
                                   ^^^^^^^^^^^^^^^^

https://github.com/openzim/zim-tools/blob/a65cb01f63d391f6684e823f5d6c3dcef3ce7fda/src/zimwriterfs/zimcreatorfs.cpp#L45

45:    Redirect redirect = {
         .path= matches[1].str(),    // designated initializers are a c++20 feature 
         .title = matches[2].str(),  // and only work prior to that using a compiler extension
         .target = matches[3].str()  // If you change warning_level=3 in meson.build
       };                            // it will show -Werror=c++20-extensions

I also found this https://github.com/openzim/zim-tools/wiki/Repology, but I am not sure if that is up to date. I am just trying to figure out which specific compiler versions are the bare minimum we want to support so it can be documented (and so I can install those versions in a container so I can test locally). Just picking out of that repology list and a cursory internet search:

Debian 10 uses GCC 10 Alpine Linux 3.16 uses LLVM 13 Ubuntu 22.04 uses GCC 11.2 Raspbian Oldstable uses GCC 8