labath marked 2 inline comments as done. labath added inline comments.
================ Comment at: lit/Modules/compressed-sections.yaml:1 +# REQUIRES: zlib +# RUN: yaml2obj %s > %t ---------------- zturner wrote: > labath wrote: > > It's right here. (I'm open to suggestions where to place it). > I see. I think part of the reason I didn't notice it is because it has a > `.yaml` extension just like the old one, so I didn't notice this was really a > test. > > LLVM is a little inconsistent here (it has tests that end in `.ll` and `.s`, > but not for most other file extensions), so can you rename this to > `compressed-sections.test`? > > At some point I think we should inject another directory in this hierarchy > (i.e. `lit/test/Modules`), but since this is not going to be the first > directory here, I guess it doesn't need to happen now. llvm (and lld) also have plenty of tests ending in .yaml. Since this is a yaml file, and plenty of editors have syntax highlighting for yaml, it seems a pitty not to take advantage of that. ================ Comment at: lit/Modules/compressed-sections.yaml:20 + - Name: .bogus +# CHECK-NOT: .bogus + Type: SHT_PROGBITS ---------------- zturner wrote: > You should probably put this as the very first check statement. Each > successfully matching `CHECK` line will update an internal position and > subsequent checks will only start from that position, so here you're only > checking that after `.bogus` does not occur after `.hello_elf`, but this test > would pass if `.bogus` occurred before `.hello_elf`. But putting the > `CHECK-NOT` first, both will fail (this is also a good reason not to > intersperse the check lines). Putting CHECK-NOT first will just make sure that .bogus does not appear *before* the first CHECK match. I put it last as it this is the place it is likely to be if it did we did end up outputting it, but if we want to be safe, I guess we have two options: - put it both at the front *and* back - have two FileCheck invocations I chose the latter. https://reviews.llvm.org/D40616 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
