[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

1

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).

11

Nit: no new line at EOF.

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

45

Nit: no new line at EOF.

[tinti]

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

1

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

1

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 ↗

(On Diff #319534)

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-windows.test

+;; Test --prefix option platform specific behavior.
+; REQUIRES: system-windows
+
+;; Test removal of trailing separators (both '/' and '\').
+;; 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
 No newline at end of file

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

 ;; Test --prefix option.
-;; UNSUPPORTED: system-windows
 
 ;; Test prefix option ignored for relative paths.
 
 ; 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
+; RUN:   FileCheck %s --check-prefix=CHECK-BROKEN-PREFIX -DFILE=%t-missing-prefix.o -DPREFIX=myprefix
 No newline at end of file