[llvm-objdump][test] Fix --prefix tests for system-windows

[tinti]

Summary

[llvm-objdump][test] Fix --prefix tests for system-windows

Merging directories and files may produce different results on different
platforms.

Merging "./Inputs" and "source-interleave-x86_64.c" will use different
separators in POSIX and Windows.

Dedicated tests are needed for dealing with removing trailing separators
for POSIX (consider only '/') and Windows (consider '/' and '\').

Fixes D85024.
Fixes PR46368.

[tinti]

Tested on Windows and Linux.

[jhenderson]

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix-windows.test

2

Could you expand this comment to explain what the platform-specific behaviour is, please? (I'm guessing it's related to the trailing separator comment below).

12

Nit: no new line at EOF.

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix.test

53

Nit: no new line at EOF.

[tinti]

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix-windows.test

2

I think I can and you are right.

Take a look at this file llvm/test/tools/llvm-objdump/X86/Inputs/source-interleave.ll

...

!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 4.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3)
!1 = !DIFile(filename: "source-interleave-x86_64.c", directory: "SRC_COMPDIR")
!2 = !{}

...

There is a directory entry and a file entry. SRC_COMPDIR will be replaced.

Merging "./Inputs" and "source-interleave-x86_64.c" will produce different results on platforms with different default separators.

Should I mention it on the code and/or commit message? Should I find where they are being merged (llc or llvm-objdump)?

[jhenderson]

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix-windows.test

2

I'm mostly interested in a comment in the test that explains why there needs to be a dedicated Windows test and why it can't be part of the general test. It can be high-level, but needs to briefly say why just doing a regex to capture slashes in either direction is insufficient.

[tinti]

Reword.
Explain behavior for adding {{[/\\]}}
Fix missing EOFs.

[jhenderson]

LGTM, with a couple of nits. Please leave it for about 24 hours to give others a chance to comment, should they wish.

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix-non-windows.test

4

system-windows is just a lit tag, and isn't the name of anything.

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix-windows.test

5

[tinti]

Reword as suggested.

[tinti]

Hi @jhenderson

Can you commit?

[jhenderson]

I'm about to sign off for the weekend, so if someone else can do it that would be good. Otherwise, I'll try to do this on Monday. Alternatively, you could always request commit access yourself?

[MaskRay]

LG

[jhenderson]

Forgot to click "Accept Revision". @tinti, have you asked for commit access?

[tinti]

In D95513#2533395, @jhenderson wrote:

Forgot to click "Accept Revision". @tinti, have you asked for commit access?

Yes I did. Thanks for the review.

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix-non-windows.test

 ;; Test --prefix option platform specific behavior.
 ; UNSUPPORTED: system-windows
 
-;; Test removal of trailing separators (only '/').
+;; Test removal of trailing separators (only '/' on non-Windows systems).
 ;; The prefix 'myprefix/\' is not changed.
 
 ; RUN: sed -e "s,SRC_COMPDIR,/Inputs,g" %p/Inputs/source-interleave.ll > %t.ll
 ; RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
 ; RUN: llvm-objdump --prefix 'myprefix/\' --source %t.o 2>&1 | FileCheck %s -DFILE=%t.o -DPREFIX='myprefix/\'
 ; CHECK: warning: '[[FILE]]': failed to find source [[PREFIX]]/Inputs/source-interleave-x86_64.c

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix-windows.test

+;; Test --prefix option platform specific behavior.
+; REQUIRES: system-windows
+
+;; Test removal of trailing separators (both '/' and '\' on Windows systems).
+;; The prefix 'myprefix/\' is changed to 'myprefix'.
+
+; RUN: sed -e "s,SRC_COMPDIR,/Inputs,g" %p/Inputs/source-interleave.ll > %t.ll
+; RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
+; RUN: llvm-objdump --prefix 'myprefix/\' --source %t.o 2>&1 | FileCheck %s -DFILE=%t.o -DPREFIX='myprefix'
+; CHECK: warning: '[[FILE]]': failed to find source [[PREFIX]]/Inputs\source-interleave-x86_64.c
+

llvm/test/tools/llvm-objdump/X86/source-interleave-prefix.test

 ;; Test --prefix option.
-;; UNSUPPORTED: system-windows
+
+;; Separators change from platform to platform. In POSIX the full path for the
+;; directory './Inputs' appended with the file 'source-interleave-x86_64.c' is
+;; './Inputs/source-interleave-x86_64.c'. For Windows it is
+;; './Inputs\source-interleave-x86_64.c'. Platform specific tests are needed
+;; since '\' may or may not be a separator.
 
 ;; Test prefix option ignored for relative paths.
+;; For the test below it is possible to accept both '/' and '\' as a separator.
 
 ; RUN: sed -e "s,SRC_COMPDIR,./Inputs,g" %p/Inputs/source-interleave.ll > %t-relative-path.ll
 ; RUN: llc -o %t-relative-path.o -filetype=obj -mtriple=x86_64-pc-linux %t-relative-path.ll
 ; RUN: llvm-objdump --prefix myprefix --source %t-relative-path.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-BROKEN-PREFIX -DFILE=%t-relative-path.o -DPREFIX=.
-; CHECK-BROKEN-PREFIX: warning: '[[FILE]]': failed to find source [[PREFIX]]/Inputs/source-interleave-x86_64.c
+; CHECK-BROKEN-PREFIX: warning: '[[FILE]]': failed to find source [[PREFIX]]/Inputs{{[/\\]}}source-interleave-x86_64.c
 
 ;; Test invalid source interleave fixed by adding the correct prefix.
 
 ; RUN: sed -e "s,SRC_COMPDIR,/Inputs,g" %p/Inputs/source-interleave.ll > %t-missing-prefix.ll
 ; RUN: llc -o %t-missing-prefix.o -filetype=obj -mtriple=x86_64-pc-linux %t-missing-prefix.ll
 ; RUN: llvm-objdump --prefix %p --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-MISSING-PREFIX-FIX
 ; CHECK-MISSING-PREFIX-FIX: ; int foo() {

 ; RUN: llvm-objdump --prefix / --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-BROKEN-PREFIX -DFILE=%t-missing-prefix.o -DPREFIX=''
 
 ;; All trailing separators on the prefix are discarded.
 ;; The prefix 'myprefix//' is converted to 'myprefix'.
 
 ; RUN: llvm-objdump --prefix myprefix// --source %t-missing-prefix.o 2>&1 | \
 ; RUN:   FileCheck %s --check-prefix=CHECK-BROKEN-PREFIX -DFILE=%t-missing-prefix.o -DPREFIX=myprefix
+