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.