[PATCH] D94668: [debug-info] [NFC] add is-a(isa<>) support for MCStreamer

Wed Feb 10 00:14:19 PST 2021

MaskRay added a comment.

In D94668#2553371 <https://reviews.llvm.org/D94668#2553371>, @shchenz wrote:

> Thanks for review @MaskRay
>
> In D94668#2553332 <https://reviews.llvm.org/D94668#2553332>, @MaskRay wrote:
>
>> `isa` in the subject confused me - I thought it was "An unsigned integer whose value encodes the applicable instruction set architecture for the current instruction" in the line number information, given the debug info context.
>>
>> It is actually https://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html
>
> Right, the `isa` in the subject is for `is-a` used in llvm style RTTI.
>
> In D94668#2553331 <https://reviews.llvm.org/D94668#2553331>, @MaskRay wrote:
>
>> In `PPCAsmPrinter.cpp`, if you use `getStreamerKind() == MCStreamer::StreamerKindAs`, then the code motion into `MCAsmStreamer.h` will not be needed. The `classof` functions are unused, too.
>
> I am not very clear about this comment, I think `isa<MCAsmStreamer>`  is more common usage. We don't need to explicitly call `getStreamerKind()` to do a type check. For example, when we check an `AllocaInst`, we won't do this like `I->getOpcode() == Instruction::Alloca`, we do it like `isa<AllocaInst>(I)`.

Both `->isXXXX` and `isa<...>` are common. `isa<MCAsmStreamer>` requires you to move the code to a separate header. I don't think it justifies the code motion.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94668