[Lldb-commits] [PATCH] D74023: [RISCV] ELF attribute section for RISC-V

Mon Mar 30 00:30:07 PDT 2020

jhenderson added a comment.

In D74023#1949261 <https://reviews.llvm.org/D74023#1949261>, @HsiangKai wrote:

> In D74023#1948388 <https://reviews.llvm.org/D74023#1948388>, @MaskRay wrote:
>
> > The code generally looks good. For unittests, I think we can either make llvm-readobj -A canonical or the unittests canonical. If we decide to place tests on one place, we should delete most tests on the other side.
> >
> > My current preference is that we use more of unittests and leave the minimum to `test/llvm-readobj/ELF/{ARM,RISCV}/`
>
>
> Agree. I will remove redundant tests in test/tools/llvm-readobj/ELF/{ARM,RISCV}/.

I also agree with testing primarily through the unit-tests where possible. However, there are probably aspects that still need to be tested via lit, namely code behaviour in ELFDumper.cpp, so just be slightly wary. High-level examples that might or might not be relevant for this patch are making sure that the output is printed correctly, and making sure errors/warnings can be threaded up to the final output.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74023