[Lldb-commits] [PATCH] D136761: [lldb][FormatEntity] Fix closing parenthesis for function.name-with-args frame format

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D136761#3885557 <https://reviews.llvm.org/D136761#3885557>, @Michael137 wrote:

> In D136761#3885529 <https://reviews.llvm.org/D136761#3885529>, @labath wrote:
>
>> Wow, another name parser I knew nothing about. :/
>>
>> I'm probably being naive, but I don't suppose there's an easy a way to reuse/repurpose the parser in the C++ language plugin for this (?) This is the first time I see this code, so it's hard to construct counter-examples, but I'd be surprised if this is correct.
>
> Hehe I agree this function could use some clean-up
> I tried to avoid going down that rabbit hole at the moment but I'll double check :)
> I wouldn't be shocked if there were edge-cases that don't work.
>
> The algorithm iiuc works as follows:
>
> 1. Find the opening function parenthesis (`open_paren`) and print out the string up to that point (effectively the function name)
> 2. For each variable in scope print out: `<var_name>=<var_representation>`
> 3. Find closing parenthesis
> 4. Print everything from closing parenthesis onward (I imagine this is to preserve function qualifiers)

Ok, so it basically takes the function base name, and then manually prints the list of function arguments inside parenthesis.  That sounds like it could be easy to do with the other parser. Keep the argument printing part, and replace the basename parsing with a call to the language parser ?

I wouldn't say its mandatory but, unlike removing the language parser, this could actually be achievable.



================
Comment at: lldb/source/Core/FormatEntity.cpp:1673
                 if (open_paren)
-                  close_paren = strchr(open_paren, ')');
+                  close_paren = strrchr(open_paren, ')');
               } else
----------------
Michael137 wrote:
> labath wrote:
> > What if there are multiple function arguments? Won't this find the end of the last one?
> Had a local test-case that worked iirc. A well-formed demangled function name always has a final closing parenthesis after all the function arguments. So unless I missed something it should find the last parenthesis correctly
> 
> I will add a test-case for this to make sure
Ok, I see. Given your description above, I believe this will work OK. However, I suspect it will go wrong in the part where it tries skip over the template arguments (and gets confused by things like `operator<<` and `foo<(2 > 1)>`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136761