zturner added inline comments.
================ Comment at: lit/Modules/compressed-sections.yaml:1 +# REQUIRES: zlib +# RUN: yaml2obj %s > %t ---------------- 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. ================ Comment at: lit/Modules/compressed-sections.yaml:12-13 + - Name: .hello_elf +# CHECK: Section 1 +# CHECK: Name: .hello_elf + Type: SHT_PROGBITS ---------------- Can you separate the `CHECK` lines and the YAML content? I think it makes it easier to follow this way, and it gives a consistent paradigm (checks first, then input, or vice versa). Interspersing them doesn't always work (for example if the tool doesn't output things in the same order as the input description). ================ Comment at: lit/Modules/compressed-sections.yaml:17-18 + Content: 010000000800000001000000789c5330700848286898000009c802c1 +# CHECK: File size: 8 +# CHECK: Data: 2030405060708090 + - Name: .bogus ---------------- Can you use `CHECK-NEXT` for these two? As it stands, if we output: ``` Name: .hello_elf File size: -1 Data: -1 Name: .hello_coff File size: 8 Data: 2030405060708090 ``` It would pass, as written. ================ Comment at: lit/Modules/compressed-sections.yaml:20 + - Name: .bogus +# CHECK-NOT: .bogus + Type: SHT_PROGBITS ---------------- 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). https://reviews.llvm.org/D40616 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
