[Lldb-commits] [PATCH] D87172: Check if debug line sequences are starting after the first code segment

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp:369
+  EXPECT_EQ(section_sp->GetType(), eSectionTypeCode);
+}
----------------
clayborg wrote:
> I would rather deal with an  C++ unit test any day. Trying to track down what set of convoluted command line commands reproduce some lit test is quite annoying and takes me a lot more time to debug. I think this test is targeted and tests what is needed. I would vote to keep this one over converting to a text dump test. My main reasoning is that it isn't possible to re-create a compilable test case that will survive any compiler that it used (past, present and future), and all symbol resolution is done bone using this call in all cases. When something goes wrong, very easy to compile the binary and debug.
If we put this up for a vote, I think you'd be in the minority. :)

I'm not sure what you find hard about reproducing a lit test -- the commands to do that get printed as a part of the test. And most of the time you don't need to run all the command to reproduce it -- running the last one suffices as the intermediate files are left over from the previous test run. I consider the leftover temporaries as one of the best aspects of this method. In this case, I could for example run llvm-dwarfdump on the intermediate object file to better understand the input that lldb gets.

Note that I am not advocating changing the test input to c++ source. I think the yaml is just fine (if I was writing it, I would probably have made that an assembler file). I just meant changing the test method by prefixing the yaml with something like:
```
# RUN: yaml2obj %s > %t
# RUN: %lldb %t -b -o "image lookup -f main.cpp -l 2" | FileCheck %s
# CHECK: LineEntry: {{.*}}main.cpp:2 # or something like that
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87172