Re: [PATCH 5/5] gdbsupport: add path_join function

[Pedro Alves <pedro@palves.net>]

On 2022-04-18 21:55, Simon Marchi wrote:
>> On 2022-04-18 15:09, Pedro Alves wrote:
>>> On 2022-04-18 19:43, Simon Marchi via Gdb-patches wrote:
>>>>> One implementation could be:
>>>>>
>>>>>     /* The real worker can be compiled in a .cc file if you wish.  */
>>>>>     extern std::string path_join_1 (std::string a, std::string b);
>>>>>
>>>>>     template <typename ...Args)
>>>>>     std::string
>>>>>     path_join (std::string a, std::string b, Args... comps)
>>>>>     {
>>>>>       return path_join (path_join (a, b), comps...);
>>>>>     }
>>>>>
>>>>>     template <>
>>>>>     std::string
>>>>>     path_join (std::string a, std::string b)
>>>>>     {
>>>>>       return path_join_1 (a, b);
>>>>>     }
>>>>>
>>>>> I do not pretend this is optimal (it is not).  In this POC I pass all
>>>>> arguments as std::string and by value, this is probably not what you
>>>>> want to do in a production grade implementation, but this just gives the
>>>>> overall idea.
>>>>>
>>>>> What do you think?
>>>> If you can make it work using gdb::string_view or const char *, I think
>>>> it would be ok.  I'll give it a try.  But otherwise, I am personally
>>>> fine with the sentinel nullptr, given that the compiler gives you a
>>>> warning if you forget it.
>>>
>>> I think it would just work as is you replace std::string with const char *
>>> in Lancelot's snippet above.  FWIW, I had the same thought as Lancelot
>>> about using variable templates instead of varargs.
>>>
>>> OOC, is there any place in GDB that wants to pass more than two components
>>> to path_join?  I skimmed the patch and didn't notice one.
>>
>> I am not aware of one, I'd be happy to make path_join just have two
>> parameters and be done with it.
> 
> Actually, I have one use for it in my upcoming DWARF reader changes,
> where I join the CU's compilation directory, the directory (from the
> line header directory table) and the file name.
> 
> I like Tom's proposal, here:
> 
>   https://sourceware.org/pipermail/gdb-patches/2022-April/188016.html
> 
> It seems efficient (no extraneous string copies) and type-safe.  I
> successfully changed my patch to use it.

Lancelot's approach seems more straightforward (trivially changed to use string_view) and
would also work, and it avoids creating the temporary array, at the expense of being
recursive when you have more than two components to join.  Either approach seems
good to me.  In Tom's approach, I think you could use gdb::array_view in concat.