MaskRay added a comment. Herald added a subscriber: sunshaoce. In D146054#4587210 <https://reviews.llvm.org/D146054#4587210>, @4vtomat wrote:
> In D146054#4586067 <https://reviews.llvm.org/D146054#4586067>, @MaskRay wrote: > >> I think the best place to test `RISCVISAInfo.cpp` is >> `llvm/unittests/Support/RISCVISAInfoTest.cpp`. >> >> `clang/test/Driver/print-supported-extensions.c` can test just a few lines >> (there will be some overlap with the testing in >> `llvm/unittests/Support/RISCVISAInfoTest.cpp`), so that changes to RISC-V >> extensions will generally not require updates to >> `clang/test/Driver/print-supported-extensions.c` > > The goal of this patch is to list the supported extensions and their versions > by providing an option, so I guess > `clang/test/Driver/print-supported-extensions.c` aims differently with > `llvm/unittests/Support/RISCVISAInfoTest.cpp` which is testing the > functionalities in `RISCVISAInfoTest.cpp`. > `clang/test/Driver/print-supported-extensions.c` only tracks the extensions > added and the their version changes and `riscvExtensionsHelp` in > `llvm/lib/Support/RISCVISAInfo.cpp` doesn't have any input or output as well > as any side effect, it only reads `SupportedExtensions` and > `SupportedExperimentalExtensions` and dump the information. > So I think `clang/test/Driver/print-supported-extensions.c` is enough for > this patch? My comment is about: the test is placed at the wrong layer. See https://maskray.me/blog/2021-08-08-toolchain-testing#the-test-checks-at-the-wrong-layer I don't want that RISC-V development in LLVM causes repeated changes to `clang/test/Driver`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146054/new/ https://reviews.llvm.org/D146054 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
