[Lldb-commits] [PATCH] D113174: [lldb] Summary provider for char flexible array members

Tue Nov 9 05:50:57 PST 2021

labath added a comment.

In D113174#3112464 <https://reviews.llvm.org/D113174#3112464>, @shafik wrote:

> In D113174#3109664 <https://reviews.llvm.org/D113174#3109664>, @jingham wrote:
>
>> Regex Type summary matching happens after the ConstString matching.  Enrico did it that way because the ConstString matching is so much quicker, so if we can find a match there we'll get to it more quickly...
>>
>> So this patch moves the char * recognition from the beginning of the type summary matching to the end, and potentially makes it a slower match.
>>
>> I doubt that this will be noticeable on modern systems, however, just something to keep in mind.
>>
>> It also changes the order of search slightly.  I think this is observable: it would mean a regex that happens to match "char *" as well as other things used to not be chosen for "char *" because it would have hit the ConstString summary first.  Now it will match, because the built-in regex summary will be checked after the user added ones.
>>
>> Again, I don't think this is a reason not to do the patch.  But something to keep in mind.  The code itself looks fine.
>
> I was looking at these char formatters a while ago and IIRC I did see some impact making these regex matches but I don't remember the details and did not look closely (so maybe there was another effect at play) b/c I went with a different approach.

This is still a bounded set, so it is possible to replace this regex with 6 strings. I did a regex because it makes the code shorter, but I have no problem with spelling this out.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113174