[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
----------------
CarlosAlbertoEnciso wrote:
> 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.
An updated patch for `09-codeview-reader` https://reviews.llvm.org/D125784 includes this change.


================
Comment at: llvm/unittests/DebugInfo/LogicalView/CMakeLists.txt:9
 
-add_llvm_unittest(DebugInfoLogicalViewTests
+add_llvm_unittest_with_input_files(DebugInfoLogicalViewTests
   CommandLineOptionsTest.cpp
----------------
CarlosAlbertoEnciso wrote:
> 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.
I was not able to rewrite the test without using `getInputFileDirectory`. Looked at other test cases that include a binary file and they use the same approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125783