[PATCH] D99967: [Flang] Changes to mangling code

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D99967#2688047 <https://reviews.llvm.org/D99967#2688047>, @kiranchandramohan wrote:

> In D99967#2687886 <https://reviews.llvm.org/D99967#2687886>, @dblaikie wrote:
>
>> 
>
> Thanks @dblaikie for highlighting the issues in this patch.
>
>> For future reference might be worth separating independent patches and giving a more specific/descriptive title. The three lines in the description seem like separate patch descriptions.
>
> OK I will separate them and improve the description in future.

Thanks! No worries.

>> Though the description mentions that the functions are all tested - yet it looks like this patch adds a new function/overload without any callers, so how is it tested?
>
> Apologies for the misunderstanding here. What i meant is that the function that is ultimately called (fir::NameUniquer::doType) is already tested and this is a wrapper around that function. I will check today if I can add a unit  
>  test for this function (assuming that is the recommendation here).

Hmm - well, then there's the question of: What's the purpose of the new function? We don't usually add new code without usage (& then test the new code via that usage). But there are some times when it's suitable to implement new code with nothing but a unit test for coverage - usually only if that code is intended for API usage by clients outside the LLVM project.

What's the purpose/intended usage of this new code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99967