[libcxx-commits] [PATCH] D92571: [IR] Remove CastInst::isCastable since it is not used

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxxabi/test/test_demangle.pass.cpp:21547
     {"_ZN4llvm8CastInst12CreateFPCastEPNS_5ValueEPKNS_4TypeERKNS_5TwineEPNS_10BasicBlockE", "llvm::CastInst::CreateFPCast(llvm::Value*, llvm::Type const*, llvm::Twine const&, llvm::BasicBlock*)"},
-    {"_ZN4llvm8CastInst10isCastableEPKNS_4TypeES3_", "llvm::CastInst::isCastable(llvm::Type const*, llvm::Type const*)"},
     {"_ZN4llvm8CastInst13getCastOpcodeEPKNS_5ValueEbPKNS_4TypeEb", "llvm::CastInst::getCastOpcode(llvm::Value const*, bool, llvm::Type const*, bool)"},
----------------
dexonsmith wrote:
> I don't understand why this is being removed. Is the demangler testcase no longer valid?
It's still valid -- the only reason why I'm not opposing to the removal is that I can see a *small* benefit in not leaving behind old definitions like that, which show up whenever you grep for the function name in the whole monorepo.

Basically, I don't mind whether we keep it or not. I think the benefit is small, but the loss of test coverage is also very small, since I assume those symbols were just taken more or less randomly and passed through `c++filt`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92571