[PATCH] D123623: [Debuginfo][llvm-dwarfutil] Add check for unsupported debug sections.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added inline comments.


================
Comment at: llvm/test/tools/llvm-dwarfutil/ELF/X86/warning-skipped-loclists.test:8
+
+# CHECK: [[FILE]]: warning: '.debug_loclists' is not currently supported: compile unit will be skipped
+
----------------
jhenderson wrote:
> dblaikie wrote:
> > Might be good to check these errors all include more info, like a file name - as it stands (what's actually FileCheck'd here) it'd be a pretty unactionable/opaque error message.
> > 
> > To do this you'd probably have to put the file in a particular location, like `%t-filename.o` then regex match in the `CHECK` line to skip the prefix and just look for `erorr message {{.*}}-filename.o` or the like.
> > 
> > Maybe it'd be simpler/more accurate for the error message to say "file will be skipped" - since I assume that's what's going to happen, yeah? Even if the file contains multiple units (could be multiple compile units from LTO, and/or multiple type units as well)
> This check does check the filename though? That's what the `[[FILE]]` bit is doing...
> Might be good to check these errors all include more info, like a file name - as it stands (what's actually FileCheck'd here) it'd be a pretty unactionable/opaque error message.
> 
> To do this you'd probably have to put the file in a particular location, like `%t-filename.o` then regex match in the `CHECK` line to skip the prefix and just look for `erorr message {{.*}}-filename.o` or the like.

The error message already contains file name.  [[FILE]] matches with file name.

> 
> Maybe it'd be simpler/more accurate for the error message to say "file will be skipped" - since I assume that's what's going to happen, yeah? Even if the file contains multiple units (could be multiple compile units from LTO, and/or multiple type units as well)

yes, I think that would be better message: "file will be skipped". Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123623