`<regex>`: After `_Uelem` changes, Build2 asserts in the Real World Code test suite

[StephanTLavavej]

After merging #5592, our test team reported VSO-2548012 "[RWC][prod/fe][Regression] Build2 test failed with Assertion failed: type () == line_type::special, file C:\gitP\Build2\build2-toolchain-0.17.0\build2\libbuild2/script/regex.hxx, line 208". The affected code is:

https://github.com/build2/build2/blob/05d68f47aa044da9161d9bfabdd153602169d902/libbuild2/script/regex.hxx#L200-L210

        // Provide basic_regex (such as from msvcrt) with the ability to
        // explicitly cast line_chars to implementation-specific numeric
        // types (enums, msvcrt's _Uelem, etc).
        //
        template <typename T>
        explicit
        operator T () const
        {
          assert (type () == line_type::special);
          return static_cast<T> (special ());
        }

I haven't yet attempted to repro this myself. I don't know if it's a bug in Build2 (i.e. a non-Standard assumption that we've broken), or a bug in #5592. I also don't know if this is further affected by the currently open #5671.

FYI @muellerj2.

[StephanTLavavej]

I have an attempted fix: build2/build2@master...StephanTLavavej:build2:regex

It looks like they were being overly sensitive about the type we're casting to, and that we're now casting to something other than char specifically.

[muellerj2]

I mentioned Build2's peculiar <regex> usage in #995 already because it's doing lots of dubious stuff (such as specializing make_unsigned).

After #5592, std::basic_regex is probably casting the character to unsigned char or unsigned int in a context it previously didn't, causing the assertion to trigger. The changes in the PRs assume that we can freely convert between the character type and unsigned int or unsigned char (up to narrowing according to the usual C++ rules) and this assertion violates this assumption because it forbids casting some values of the character type in this way.

The operator char() implementation suggests that this could be an appropriate fix, I guess. But there is some chance that this will be immediately followed by another assertion because '\a' does not seem to be among the allowed group of special characters and we are checking for narrowing using round-trip casting: https://github.com/build2/build2/blob/05d68f47aa044da9161d9bfabdd153602169d902/libbuild2/script/regex.cxx#L44-L76

It's difficult for me to say more without setting up the project and debugging this myself. Unfortunately, I'm quite busy currently and doubt I will have time to take a closer look at it before Sunday, if at all this week.

[StephanTLavavej]

Yeah, my attempted fix just causes another build2 assertion:

Assertion failed: c == 0 || c == -1 || c == u'\u2028' || c == u'\u2029' || (c > 0 && c <= 255 && ( syntax (c) || string::traits_type::find (ex, 4, c) != nullptr)), file C:\gitP\Build2\build2-toolchain-0.17.0\build2\libbuild2\script\regex.cxx, line 75

I'll investigate this week (after doing a toolset update).

[muellerj2]

This suggests that this is triggered by some newly introduced round-trip cast that checks whether a character fits into 8 bits.

I suspect that adding \a to ex in this line is good enough: https://github.com/build2/build2/blob/05d68f47aa044da9161d9bfabdd153602169d902/libbuild2/script/regex.cxx#L55

But the existence of ex with all of these exceptions for libstdc++'s regex engine just shows how incredibly brittle all these assertions are.

[muellerj2]

Or it's even more trivial: The parser now casts ASCII characters to the character type before comparing for (in-)equality. But the assertion fails on many ASCII characters that can have special meaning in regexes, such as quite a few letters that appear in escape sequences like \b, \B, \cA, \W etc.