bulbazord added a comment.

In D108395#2957194 <https://reviews.llvm.org/D108395#2957194>, @teemperor wrote:

> I'm very supportive of the idea that we delete everything that isn't tested 
> :) (and I'm only partly joking here).

I can sympathize, I'm only partly serious with this change! :D

> However removing this should cause a bunch of test failures on Linux as it 
> effectively disables D70721 <https://reviews.llvm.org/D70721> (the test added 
> in that patch was just a unit test that is preserved with this patch, but the 
> bigger problem describes there will break a few tests calling constructors on 
> Linux).

It doesn't cause test any test failures on my Linux machine unfortunately. I 
don't think this codepath is currently exercised by any tests. Certainly the 
`FindAlternateFunctionManglings` test in `CPlusPlusLanguageTest.cpp` also uses 
`CPlusPlusLanguage::FindAlternateFunctionManglings`, but that's not the same 
thing as testing `IRExecutionUnit::CollectCandidateCPlusPlusNames` being tested 
as a part of `IRExecutionUnit::FindSymbol`.

> I believe the usual 'iterate over all language plugins' approach would again 
> work here? We can guess from the mangled symbol what language we deal with 
> (and in the worst case we could encode that in the IRExecutionUnit), so the 
> plugins would just deliver alternative names. 
> `GetAlternativeSymbolNames(ConstString symbol)` or something like that maybe?

I definitely think that the usual "iterate over all language plugins" approach 
would work here if we don't delete this (which it seems like we shouldn't). I 
mostly uploaded this change to see if anybody knew how to trigger this 
codepath. Before refactoring I'd like to write a test for this codepath, though 
I'm still not entirely sure how to do that yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108395

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to