[PATCH] D125783: [llvm-debuginfo-analyzer] 08 - ELF Reader

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/unittests/DebugInfo/LogicalView/CMakeLists.txt:3
+  AllTargetsDescs
+  AllTargetsInfos
+  AllTargetsDisassemblers
----------------
thakis wrote:
> (alphabetize?)
There is only one remaining patch (09-codeview-reader) for the `llvm-debuginfo-analyzer` tool; it changes this file to include the `CodeViewReaderTest.cpp`.

I will address the `(alphabetize?)` issue in that patch.


================
Comment at: llvm/unittests/DebugInfo/LogicalView/CMakeLists.txt:9
 
-add_llvm_unittest(DebugInfoLogicalViewTests
+add_llvm_unittest_with_input_files(DebugInfoLogicalViewTests
   CommandLineOptionsTest.cpp
----------------
thakis wrote:
> See:
> 
> ```
> # Use for test binaries that call llvm::getInputFileDirectory(). Use of this
> # is discouraged.
> function(add_unittest_with_input_files test_suite test_name)
> ```
> 
> Any chance you could write this test without needing getInputFileDirectory()?
There is only one remaining patch (09-codeview-reader) for the `llvm-debuginfo-analyzer` tool; it changes this file to include the `CodeViewReaderTest.cpp` which uses the `getInputFileDirectory` call.

I will explore the option to remove the call to `getInputFileDirectory` in that patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125783