Page MenuHomePhabricator

[llvm-objdump] Remove unnecessary indentation when dumping ELF data.
ClosedPublic

Authored by ychen on Jun 16 2019, 6:01 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

ychen created this revision.Jun 16 2019, 6:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2019, 6:01 PM
ychen edited the summary of this revision. (Show Details)Jun 16 2019, 6:07 PM
ychen retitled this revision from [llvm-objdump] remove unnecessary indentation when dumping ELF data. to [llvm-objdump] Remove unnecessary indentation when dumping ELF data..

You can edit test/tools/llvm-objdump/X86/disassemble-code-data-mix.s to check this. X86/disassemble-align.s is an example to check \t.

ychen added a comment.Jun 16 2019, 7:12 PM

You can edit test/tools/llvm-objdump/X86/disassemble-code-data-mix.s to check this. X86/disassemble-align.s is an example to check \t.

Thank you! Will do.

ychen updated this revision to Diff 204988.Jun 16 2019, 7:34 PM
  • add test
MaskRay accepted this revision.Jun 16 2019, 8:22 PM
MaskRay added inline comments.
llvm/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s
17 ↗(On Diff #204988)

Assume you'll update this part when committing...

This revision is now accepted and ready to land.Jun 16 2019, 8:22 PM
ychen marked 2 inline comments as done.Jun 16 2019, 10:13 PM
ychen added inline comments.
llvm/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s
17 ↗(On Diff #204988)

I thought I don't need to. Without this change, the modified test should fail.

// CHECK: b: 74 65 73 74 20 73 74 72 test str

^

<stdin>:2:1: note: scanning from here
<stdin>:|file format ELF64-x86-64
^
<stdin>:15:2: note: possible intended match here
b:| 74 65 73 74 20 73 74 72 test str
^

MaskRay added inline comments.Jun 16 2019, 10:26 PM
llvm/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s
17 ↗(On Diff #204988)

ok, you probably need FileCheck --strict-whitespace

jhenderson added inline comments.Jun 17 2019, 4:47 AM
llvm/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s
17 ↗(On Diff #204988)

There are two related switches here that are important to note. I haven't inspected the test enough to figure out which are the right ones to use here. The default behaviour is for FileCheck to ignore leading and trailing whitespace, and to replace all contiguous whitespace with a single space, both in the CHECK lines and in the text it is checking again. --strict-whitespace overrides the latter of these, but not the former, whilst --match-full-lines overrides the former but not the latter. Using both means you get an exact match of the line, whitespace and everything, which is usually what you want when you are testing formatting (though you can also use regexes to achieve some of the same thing). One gotcha with doing both is that if you do this, you can't have a space character between any CHECKs and the first character you are checking, unless your line starts with a space.

Also, watch out for hard tabs, since editors will often get rid of those. If checking hard tabs, use {{\t}} explicitly (together with --strict-whitespace).

ychen updated this revision to Diff 205227.Jun 17 2019, 6:07 PM
ychen marked 4 inline comments as done.
  • update test
ychen added inline comments.Jun 17 2019, 6:07 PM
llvm/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s
17 ↗(On Diff #204988)

Thank you @MaskRay @jhenderson. That's really helpful.

I used tr '\t' '|' to make sure there are no tabs in the output for this test.

I don't think you need the tr bit. If you have any tabs, the FileCheck will fail, because the whitespace in the lines doesn't contain any tabs.

ychen updated this revision to Diff 205348.Jun 18 2019, 7:51 AM
  • update
ychen added a comment.Jun 18 2019, 7:51 AM

I don't think you need the tr bit. If you have any tabs, the FileCheck will fail, because the whitespace in the lines doesn't contain any tabs.

Agree. Test updated.

This revision was automatically updated to reflect the committed changes.