[Lldb-commits] [PATCH] D66654: Prevent D66398 `TestDataFormatterStdList`-like regressions in the future

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D66654#1645792 <https://reviews.llvm.org/D66654#1645792>, @jankratochvil wrote:

> In D66654#1645658 <https://reviews.llvm.org/D66654#1645658>, @clayborg wrote:
>
> > What about having the first one that matched being the one that is used and avoid errors all together?
>
>
> This is how it worked so far before the fix D66398 <https://reviews.llvm.org/D66398> (which made the regexes unambiguous - always matching at most 1 regex for any string).  The previous version of the patch <https://reviews.llvm.org/D66398?id=215895> just sorted the regexes alphabetically because original LLDB did not sort them at all printing random results.
>
> @labath did not like the alphabetical sorting as (IIUC) adding formatters from a new library can affect formatting for a different library.


Kind of, yes. My concern was a bit more general, in that the alphabetical sorting just seems arbitrary and I don't believe anybody would expect that. It happens to do what we want here, but I can imagine that in some cases it might do the exact opposite, and then we'd have to massage the regular expressions to reverse their sort order, but retain meaning...

Personally, I'd go for the registration order, or possibly the reverse registration order, depending on what we want to give precedence to. It seems to me that this would give the most predictable results, and would make it easy to swap the precedence of two formatters if needed. But I don't feel strongly about that, so if you want to go with alphabetical, then I guess that's fine with me (it's definitely better than sorting on pointer values :P).


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66654/new/

https://reviews.llvm.org/D66654