[PATCH] D73383: Allow retrieving source files relative to the compilation directory.

Mon Jan 27 10:39:18 PST 2020

dblaikie added a comment.

In D73383#1842442 <https://reviews.llvm.org/D73383#1842442>, @saugustine wrote:

> The enum is present only in an assert because as the code is written, it works as, an "other" case. It is something a client would pass, and right now there are no clients inside llvm. I guess I could add one to the symbolizer.

Yeah, makes sense to me.

> As I mentioned in the original message, I would love to add a test case for this, but I'm not sure where. The rest of the code isn't explicitly tested except by indirectly by way of symbolizer tests. So I guess I can go that route.

Right - LLVM has few API level unit tests (though there are some for libDebugInfoDWARF in llvm/unittests/DebugInfo/DWARF/ for instance), most testing is done by exposure through command line tools.

> What flag would you like? Absolute is the symbolizer default, and "-s" makes it report basenames only.

Doesn't have to be super brief/punchy if we don't have a normal/common/regular use for it. I'd just go with "-no-comp-dir" for now.

In D73383#1839761 <https://reviews.llvm.org/D73383#1839761>, @saugustine wrote:

> This revision also moves the absolute path logic out of the dwarf-4 only side of the conditional. I believe that was a mistake in an earlier patch, because there is nothing special about dwarf-4 vs dwarf-5 in this regard. Although there are some changes in where it is stored, they aren't relevant to whether it should be appended.

That might be worthy of a separate patch & test case demonstrating the bug that's being fixed by moving that code out of the conditional.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73383