July 21, 2022
12:24 p.m.
On Thu Jul 21 15:31:48 2022 +0000, eric pouech wrote:
> You got me...
> actually I had another test in first version of the patch
> /* xxx */ {"??Bcastop@@QAE?BHXZ", "public: __thiscall castop::operator
> int const (void)"},
> but this failed because of erroneous space character in the output (a
> space is inserted after the const, while none is inserted if there's no
> qualifier as in the test added to this patch). I removed the test and
> the rest of the changes to support it as I couldn't get it right in a
> decent way (ie
> for the record I'm also running undecoration code against:
> A) a list of C++ symbols gotten from various pdb files
> B) a list of tests imported from llvm test suite... (in a bunch of cases
> llvm's output differs from native msvcrt as well)
> and comparing it against native msvcrt output
> apart from a lot of undecoration errors in A) & B), I've got also a
> bunch of white space only discrepancies
> (and to link to test case above, there's some other cases where we're
> missing a white space after a non empty qualifier to a type); but I
> currently gave up to understanding where this comes from
> I'm uncertain how to tackle this kind of issue:
> - fixing space for space the output looks not easy at all... and doing
> it at the right place even more (I don't like some of the ugly tests -
> for example: search for /* don't insert a space between duplicate '*' */)
> - mark the test as todo_wine; but shall we consider generating "operator
> int const(void)" instead of "operator int const (void)" as an error?
> both are valid C++ constructs, and both correspond to the mangled symbol
> - add a new mark for wine white space only changes (actully around some
> delimiters - likely "<>()*,"
If you're looking for some hints I'm afraid I don't have any. You're much more familiar with this code than I am. I would probably try to do it this way:
- check if ucrtbase produces the output in the same way (usually it's better to follow ucrtbase output if there's difference)
- try to implement it so the output is identical as in native
- if it's hard or not possible output something that is valid C++ (but in this case there should be a test that fails)
I'm not sure if we need any special support for it in tests. Probably adding the tests and marking it as todo_wine is good enough. Another option is to introduce something like strcmp_ignore_ws function (that compares all the characters except ' ') and use it for tests marked as white space failures. If anything like strcmp_ignore_ws is introduced, I think it should be used in following way:
```
todo_wine_if(test[i].flags & TODO_WINE_WS) ok(!strcmp(test[i].out, name), ...);
if (test[i].flags & TODO_WINE_WS)
ok(!strcmp_ignore_ws(test[i].out, name), ...);
```
I will take a look on ** case to see if I can find an elegant solution for it. I'll let you know if I find anything.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/492#note_4692