[PATCH] D74169: [LLD][ELF][DebugInfo] Skeleton implementation of removing obsolete debug info.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

`test/ELF/gc-debuginfo.s` works without the code change (after deleting `--gc-debuginfo`). How many changes do you expect will be needed to make the feature work?



================
Comment at: lld/ELF/LLDDwarfLinker.cpp:70
+
+  // for each object file
+  for (InputFile *file : objectFiles) {
----------------
Capitalize.

Append full stop.


================
Comment at: lld/ELF/LLDDwarfLinker.cpp:70
+
+  // for each object file
+  for (InputFile *file : objectFiles) {
----------------
MaskRay wrote:
> Capitalize.
> 
> Append full stop.
`// for each object file` does not convey more information than the code itself.

Say what this loop is supposed to do.


================
Comment at: lld/ELF/LLDDwarfLinker.cpp:73
+    if (ObjFile<ELFT> *obj = cast<ObjFile<ELFT>>(file)) {
+
+      dwarfContexts.push_back(std::unique_ptr<DWARFContext>(
----------------
Delete empty line


================
Comment at: lld/ELF/LLDDwarfLinker.cpp:81
+      objectsForLinking.push_back(
+          std::unique_ptr<DwarfLinkerObjFile>(new DwarfLinkerObjFile(
+              file->getName(), dwarfObjFile, emptyWarnings)));
----------------
Doesn't make_unique work?


================
Comment at: lld/test/ELF/gc-debuginfo.s:2
+# REQUIRES: x86
+	
+# RUN: echo '.global _start; .global main;  _start: jmp main; main: ' \
----------------
Delete (invisibile) whitespace


================
Comment at: lld/test/ELF/gc-debuginfo.s:6
+
+# RUN: ld.lld -gc-sections -gc-debuginfo %t.o -o %t1.out
+
----------------
`-` -> `--`


================
Comment at: lld/test/ELF/gc-debuginfo.s:10
+
+# CHECK: file format ELF64-x86-64 
----------------
Append `# CHECK-NOT: {{.}}`


================
Comment at: lld/test/ELF/gc-debuginfo.s:10
+
+# CHECK: file format ELF64-x86-64 
----------------
MaskRay wrote:
> Append `# CHECK-NOT: {{.}}`
Delete trailing space


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74169