gcc-4.8 support

[fargies]

Hello,

I wanted to give a try to RapidYaml at work, eventually using it for production, thus had to make changes to support old compilers (gcc-4.8).

• I've added some polyfills (for 4.8 missing pieces) in c4core : fargies/c4core@master...fargies:compat-gcc-4.8
• Injected it in gtest (and fyaml) : fargies/cmake@master...fargies:compat-gcc-4.8
• Fixed some tests (explanations in commits) : fargies/rapidyaml@master...fargies:compat-gcc-4.8

Tests are passing on CentOS7 and ubuntu (I've added 4.8 in your ubuntu CI).

Those modifications felt a bit too intrusive for a pull-request, but if you wish to integrate some parts or make a review I can create some ?

PS: thanks for this awesome library 🎉 !

[biojppm]

Thanks for the kind words!

I'd be happy to merge this in, but with some high-level caveats. To save some work, let's address them here, as some of the PRs might not be needed.

• Good stuff with the 4.8 shim for c4core. There may be a couple of minor remarks, but the idea is on point. Additionally:
  • Please add the shim header also to the c4core/tools/amalgamate.py script
  • Please make a note in c4core/changelog/current.md
• I am less sure of the cmake changeset. Why the need for injection into gtest? Once you #include <c4/compiler.hpp> in client code, then nothing else should be required. Or am I missing something? Maybe that's needed not because of c4core or rapidyaml, but to be able to compile gtest with 4.8?
• In the ryml changeset, there will be a couple of minor points to address in the review. Here I'd like to address the need for this and the following diffs. I'm a bit uneasy with this. Is there no alternative? Does 4.8 really not accept multiline raw strings as macro arguments?

[fargies]

For gtest that's it, it also internally uses is_trivially_copyable.

For the multi-lines string in macros, a simple test reveals it:

#define TEST(x)

int main() {
    TEST(R"(
)");
    return 0;
}

Will produce this:

$ g++ test.cpp
test.cpp:5:11: warning: missing terminating " character [enabled by default]
     TEST(R"(
           ^
test.cpp:6:2: warning: missing terminating " character [enabled by default]
 )");
  ^
test.cpp:5:5: error: missing terminating " character
     TEST(R"(
     ^

If you'd prefer to keep the same syntax we can eventually introduce temporary callable objects to store line/file, like here.

PS: I'll have a look for the Python part, thanks for your fast reply

[biojppm]

If you'd prefer to keep the same syntax we can eventually introduce temporary callable objects to store line/file, like here.

Indeed I'd prefer to keep the syntax. The callable object may be a good approach; let's use it only where needed and preferably keep the name similar to EXPECT_EQ(). Say, EXPECT_EQ_MULTILINE(), with a note explaining that gcc 4.8 cannot accept multiline literals in the macro.

So do send the PRs and we'll address each one in more detail, starting with cmake->c4core->ryml (sorry for the complexity!).

[fargies]

I've reworked and sent you a first part (c4core) here: biojppm/c4core#68

cmake update shouldn't be needed (it felt awkward to link cmake->c4core through the -include)

[biojppm]

@fargies do you intend to proceed with the PR to rapidyaml? I'm about to make a release, but I can hold a couple of days to get that in.

[fargies]

@biojppm sure, I'll see if I can fit this tomorrow

[biojppm]

Fixed in #217