kwk added a comment.
@jankratochvil thanks for this thorough review. I have to think about one
comment more precisely but the rest was fixed.
================
Comment at: lldb/source/Host/common/DebugInfoD.cpp:50
+ "invalid build ID: %s",
+ buildID.GetAsString("").c_str());
+
----------------
jankratochvil wrote:
> It should not be an error:
> ```
> echo 'int main(void) { return 0; }' >/tmp/main2.c;gcc -o /tmp/main2
> /tmp/main2.c -Wall -g -Wl,--build-id=none;rm
> /tmp/main2.c;DEBUGINFOD_URLS=http://localhost:8002/ ./bin/lldb /tmp/main2 -o
> 'l main' -o q
> (lldb) target create "/tmp/main2"
> Current executable set to '/tmp/main2' (x86_64).
> (lldb) l main
> warning: (x86_64) /tmp/main2 An error occurred while finding the source file
> /tmp/main2.c using debuginfod for build ID A9C3D738: invalid build ID:
> A9C3D738
> File: /tmp/main2.c
> (lldb) q
> ```
>
Okay, I'll have it return just an empty string. And adjust the comment on the
empty string in findSource documentation. I fully understand that an error is
undesirable in your test case. My question is if the caller should sanitize
it's parameters passed to `findSource` of if the latter should silently ignore
those wrong UUIDs. For now I silently ignore them and treat a wrong build ID
like a not found (e.g. empty string is returned).
================
Comment at: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp:103
+// TEST-3: File: /my/new/path/test.cpp
+// TEST-3: 123
+// TEST-3-NEXT: {{[0-9]+}} // Some context lines before
----------------
jankratochvil wrote:
> `s/123/{{[0-9]+}}/?`
Both are fine, but I'll go with your's if that helps. If you can tell me how to
get a lit `CHECK` statement that checks for incremental numbers, that'll be
awesome ;)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75750/new/
https://reviews.llvm.org/D75750
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
