[CI] Fix regression tests

[bluetarpmedia]

Update a couple of test-results caused by line number changes in cpp2util.h

• gcc-13-c++2b/mixed-bugfix-for-ufcs-non-local.cpp.output
• msvc-2022-c++20/pure2-assert-expected-not-null.cpp.output

[DyXel]

I am curious, is there something we could do to avoid these repeating PRs? My only guess is that it's only possible if Herb decides to do every change through a PR (and thus run the CI before merging), which might not sound amusing to him 😅

[hsutter]

I am curious, is there something we could do to avoid these repeating PRs? My only guess is that it's only possible if Herb decides to do every change through a PR (and thus run the CI before merging), which might not sound amusing to him 😅

Interesting... so if I did all my commits as PRs, that would update the regression tests CI? (I appreciate the help with the CI but don't myself grok its inner workings.)

[DyXel]

CI runs all the available compilers and creates git patches that you can download and apply locally, you can apply those changes in that same unmerged PR and avoid "breaking" the main branch.

Another option I thought of, is to account for your current workflow and create a bot that automatically opens these kind of PRs after commits have been pushed to main. This would have less friction for you, and also don't have users wasting valuable time updating tests every time. But of course we would actually need to write and deploy that bot.

[hsutter]

create a bot that automatically opens these kind of PRs after commits have been pushed to main

That sounds promising, and would give a way to review the impact, albeit post-commit.

A third option would be to check in only the tests that I run myself (including that I run them on others' PRs before merging): Those are currently MSVC latest, GCC 10 and 14, and Clang 12, and for completeness I could add Clang 18. That would cover the lowest-supported and pretty-most-recent versions of GCC and Clang. The major thing missing is it would not cover Apple Clang, which was the original motivation to check in additional builds I wasn't doing myself (because it requires a Mac, and I'm not currently set up for that)... but if I always did { MSVC, GCC 10, GCC latest, Clang 12, and Clang latest }, and in addition we only had Apple Clang latest test results supplied some other way, that would still reduce the number of files and keep all but one of the regression tests accurate.

WDYT?

[DyXel]

Personally I don't have strong feelings one way or another, and I think this is more of a question for @bluetarpmedia and co., to me it just feels kind of bad seeing them open a PR to update tests after every change. #1137 should help with that, since only "compiler negative cases" (that is, cases that should fail to compile on the C++ compiler) would need to be updated.

[bluetarpmedia]

I've been thinking about a couple things but hadn't put them down in writing yet so this is a good opportunity: First, I've been wondering if we should drop some of the compiler configurations for the regression tests, firstly to simplify the amount of files but secondly because I'm not sure we're getting much value from some of them.

Here's what I'm thinking:

Compiler	C++ Standard	Comment
Apple Clang 14	C++2b	✅ Keep: Min supported Apple Clang? (I can't remember if cppfront works with earlier versions, or maybe it's the min supported on GitHub runners)
Apple Clang 15	C++2b	❌ Drop: Doesn't seem to provide much value compared to above
Clang 12	C++20	✅ Min supported Clang in C++20 mode
Clang 15	C++20	❌ We already test C++20 with Clang 12 above
Clang 15 with libc++	C++20	❌ This is currently the only Ubuntu Clang test using libc++, but I'd drop it in favour of changing Clang 18 below.
Clang 18	C++20	✅ Change this to C++2b and use the libc++ standard library
GCC 10	C++20	✅ Min supported GCC in C++20 mode
GCC 13	C++2b	❌ Doesn't appear to provide much value compared to GCC 14 below
GCC 14	C++2b	✅
MSVC 2022	C++20	✅ Min supported MSVC in C++20 mode
MSVC 2022	C++latest	✅

So after the change there'd be:

• Apple Clang 14 (C++2b)
• Clang 12 (C++20) and Clang 18 (C++2b and libc++)
• GCC 10 (C++20) and GCC 14 (C++2b)
• MSVC (C++20) and MSVC (C++latest)

I think that's close to what Herb suggested above, except I think it's also valuable to test the min supported MSVC in C++20 mode and not only the C++latest?

---

The other thing that's been on my mind is regarding the git diff conflicts we get with cpp2util.h when the only difference is that the line and/or column number has changed.

What do you think about stripping out the line & column numbers from the compiler output before running the git diff (and also committing that version of the output sans numbers as the expected results)?

So lines like this:

../../../include/cpp2util.h:10005:33: error: expected unqualified-id

would be edited to:

../../../include/cpp2util.h: error: expected unqualified-id

and then run the git diff on that.

[DyXel]

First, I've been wondering if we should drop some of the compiler configurations for the regression tests, firstly to simplify the amount of files but secondly because I'm not sure we're getting much value from some of them.

I guess in principle is not that bad of an idea: Just test the minimally supported compiler set and hope implementers didn't change anything or introduce a regression. My problem with this is that the amount of of files is not really about the target count, but about our current regression test implementation.

For the 2nd, I am not too sure... Its reasonable to think that if you change the file and now it errors out in a different place, that you'd need to refresh the regression tests, to me the problem isn't the conflict, is that its maybe too cumbersome to update them at the moment.

[gregmarr]

Interesting... so if I did all my commits as PRs, that would update the regression tests CI? (I appreciate the help with the CI but don't myself grok its inner workings.)

Since this wasn't answered directly:

No, it would just tell you that the tests failed in the PR branch instead of in main, just as if someone else submitted a change. Someone would still need to fix the tests by applying the patch, if that's the correct fix, or by fixing the code. It just puts that "update code, run tests, then fix tests or code and rerun tests until they all pass" loop in a branch instead of in main.