[PATCH] D103250: [llvm-dwarfdump][test] Add missing dedicated tests for some options
Owen Reynolds via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 28 08:15:19 PDT 2021
gbreynoo marked 4 inline comments as done.
gbreynoo added inline comments.
================
Comment at: llvm/test/tools/llvm-dwarfdump/X86/parent_recurse_depth.s:22
+
+.section .debug_abbrev,"", at progbits
+.Lsection_abbrev:
----------------
Higuoxing wrote:
> I think the following yaml code is more readable than raw assembly code. What do you think?
>
> ```
> --- !ELF
> FileHeader:
> Class: ELFCLASS64
> Data: ELFDATA2LSB
> Type: ET_EXEC
> Machine: EM_X86_64
> DWARF:
> debug_abbrev:
> - Table:
> - Tag: DW_TAG_compile_unit
> Children: DW_CHILDREN_yes
> Attributes:
> - Attribute: DW_AT_producer
> Form: DW_FORM_string
> - Tag: DW_TAG_subprogram
> Children: DW_CHILDREN_yes
> Attributes:
> - Attribute: DW_AT_name
> Form: DW_FORM_string
> - Tag: DW_TAG_namespace
> Children: DW_CHILDREN_yes
> Attributes:
> - Attribute: DW_AT_name
> Form: DW_FORM_string
> - Tag: DW_TAG_base_type
> Children: DW_CHILDREN_no
> Attributes:
> - Attribute: DW_AT_name
> Form: DW_FORM_string
> debug_info:
> - Version: 4
> Entries:
> - AbbrCode: 1
> Values:
> - CStr: by_hand
> - AbbrCode: 2
> Values:
> - CStr: main
> - AbbrCode: 3
> Values:
> - CStr: test
> - AbbrCode: 4
> Values:
> - CStr: int
> ```
Much better thanks.
================
Comment at: llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:638
ToolOutputFile OutputFile(OutputFilename, EC, sys::fs::OF_TextWithCRLF);
- error("Unable to open output file" + OutputFilename, EC);
+ error("Unable to open output file " + OutputFilename, EC);
// Don't remove output file if we exit with an error.
----------------
Higuoxing wrote:
> jhenderson wrote:
> > Whilst you're changing this message, I'd also change `Unable` to `unable`, in line with the LLVM error message style policy.
> I think it might be good to surround file names with single quotes `'`. e.g.
>
> ```
> Unable to open output file '/path/to/file': permission denied
> ```
I'd have to check if OutputFilename is empty or an error like `unable to open output file '': no such file or directory` can be output which looks strange IMHO . Would it better to leave it as is?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103250/new/
https://reviews.llvm.org/D103250
More information about the llvm-commits
mailing list