On Tue Jul 26 11:26:36 2022 +0000, Piotr Caban wrote:
I generally like your idea but I have few comments:
- I also don't like the %t format, I think it would be best if you find
a way to avoid it (I see it as (maybe) useful way of adding hacks that would be nice to avoid)
- I don't like handle_trailing_ws helper design, I don't think the
decision should be made basing on the character value (it adds some hidden restrictions about str_printf usage e.g. in case of differentiating between operator* and pointer) Maybe the code can be changed in a way that "optinal white space" affects only %s parameter instead (so it's closer to your initial patch but adds trailing WS instead of leading WS and preserves the OPTIONAL_WS tag between str_printf calls)?
just to summarize: - we need to define when inserting spaces (or not) between <X> and <Y> - current codes tries to guess from <X> and <Y> output, but it's fragile, doesn't cover all cases - we tried with black magic on <X> and <Y> (with RHS and LHS optional ws); but it hides the reason why it has to be inserted or not and lacks potential flexibility - we tried with starting character of <Y>; it hides the reason why is has to be inserted, and lacks potential flexibility - furthermore, + we have cases (like the consecutive '*') where it's inner type (ptr to function) that controls outter type (pointer to) for not inserting a char + but we have cases, where it's the outter type (cast operator to) which controls whether a ws has to be inserted or not (cast operators)
I only see two options: - pass information along to control the ws output (up & down unfortunately) - rewrite all the type handling code to decouple parsing from synthetizing output and let caller do what they want
second option allows any possible flexibility but at the cost of rewriting / changing all the type handling code in undname.c. Looks like a heavy hammer for a couple of changes it'll also put the hacks where we need them, whereas first option will push that complexity/operation to other places (even if we tried to make them as generic as possible)
given the size of the change, I tried first option by passing up (in datatype_t) and down (as flags to internal helpers) here's the series that tackle: - the '*' consecutive handling - the templates in ctor/dtor/castop (needed for next one) - the cast operator support (including correct ws support)
using this, it'll (likely) be: - we don't need ws_wine_broken support in tests - should cover a couple of ws issues (at least the ones that are ok to fix...)
please let me known what you think of this...[err](/uploads/f480690aab88eea594938134c5872174/err)