hokein added a comment. Thanks for the comment.
> unit test which checks llvm::writeOutput may be added to > raw_ostream_test.cpp. Let`s this test check the case when "all_exec" is not > set. i.e. that after default usage of llvm::writeOutput the resulting file > does not have "all_exec", which is exact purpose of this patch. OK, added one. ================ Comment at: llvm/test/tools/llvm-dwarfutil/ELF/X86/file-permissions.test:1 +# RUN: yaml2obj %p/Inputs/common.yaml -o %t.o +# RUN: chmod a+x %t.o ---------------- jhenderson wrote: > hokein wrote: > > jhenderson wrote: > > > It might make sense to reuse the test code from llvm-objcopy's > > > mirror-permissions tests. See my other inline comment too. > > by reusing the code, I think you mean calling the `llvm-dwardfutil` tool in > > the `llvm-objcopy` lit test, I'm not sure it is a good idea, calling a > > different tool in the > > `llvm/test/tools/llvm-objcopy/mirror-permissions-*.test` seems like > > violating the layer. > I meant that you could copy and adapt the existing llvm-objcopy test to > create a new llvm-dwarfutil test (in the llvm-dwarfutil test folder). I see, adjusted the test per comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153652/new/ https://reviews.llvm.org/D153652 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
