Page MenuHomePhabricator

[clang][deps] NFC: De-duplicate clang-cl tests
Changes PlannedPublic

Authored by jansvoboda11 on Mar 16 2022, 8:28 AM.



In D92191, a bunch of test cases were added to check clang-scan-deps works in clang-cl mode as well.

We don't need to duplicate all test cases, though. Testing the few special cases we have in clang-scan-deps for clang-cl should be good enough:

  1. Deducing output path (and therefore target name in our make output).
  2. Ignoring -Xclang arguments in step 1.
  3. Deducing resource directory by invoking the compiler executuable.

This test de-duplicates the extra clang-cl test cases.

Diff Detail

Event Timeline

jansvoboda11 created this revision.Mar 16 2022, 8:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 8:28 AM
jansvoboda11 requested review of this revision.Mar 16 2022, 8:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 16 2022, 8:28 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
jansvoboda11 edited the summary of this revision. (Show Details)Mar 16 2022, 8:57 AM

Remove backslashes from make output checks

Add -NEXT to a check

saudi accepted this revision.Mar 22 2022, 1:38 PM

It is indeed much cleaner this way`, thanks!
The tests are not anymore the same for clang-cl as for clang, but I agree what needs testing is the clang-cl specific behaviors, not having the same tests redone as a clang-cl flavor.


I was wondering whether it could be a concern that this test will be skipped on Windows systems, where clang-cl specific development would most likely occur.

However I'm pretty sure it's ok, since this test is pretty simple and small, has little chances to break during most development iterations, and we would run the tests under linux at some point anyway.

This revision is now accepted and ready to land.Mar 22 2022, 1:38 PM
saudi added a comment.Mar 22 2022, 1:40 PM

Sorry, I flagged this as "accepted", but I'm not sure I am allowed to do that...

dexonsmith accepted this revision.Mar 22 2022, 7:18 PM

LGTM too!

jansvoboda11 added inline comments.Mar 23 2022, 7:29 AM

I agree it's not great, but we need to be able to create an executable here (to simulate clang -print-resource-dir). Since Windows doesn't have the concept of shebangs, I don't think there's a way to make this work. I remember also trying to achieve this by creating a .py script and relying on Windows' "run .py files with the Python interpreter" rule. Unfortunately, that doesn't kick in when we run the command from within Clang.

This page says Git for Windows with bash tools is required for building LLVM, so I think it's reasonable to expect REQUIRES: shell will pass for a lot of Windows developers. Is that not the case?

This revision was landed with ongoing or failed builds.Mar 23 2022, 7:42 AM
This revision was automatically updated to reflect the committed changes.
gulfem added a subscriber: gulfem.Mar 23 2022, 3:34 PM

Newly added tests ClangScanDeps/cl-output.c and ClangScanDeps/cl-xclang.c started failing in our Mac toolchain builds:
Here's the link to the log:

The failure is as the following:

FAIL: Clang :: ClangScanDeps/cl-xclang.c (2170 of 30232)
******************** TEST 'Clang :: ClangScanDeps/cl-xclang.c' FAILED ********************
: 'RUN: at line 4';   rm -rf /opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp
: 'RUN: at line 5';   split-file /opt/s/w/ir/x/w/llvm-llvm-project/clang/test/ClangScanDeps/cl-xclang.c /opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp
: 'RUN: at line 16';   sed -e "s|DIR|/opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp|g" /opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp/cdb.json.template > /opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp/cdb.json
: 'RUN: at line 17';   /opt/s/w/ir/x/w/staging/llvm_build/bin/clang-scan-deps -compilation-database /opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp/cdb.json > /opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp/result.d
: 'RUN: at line 18';   cat /opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp/result.d | sed 's:\\\\\?:/:g' | /opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck /opt/s/w/ir/x/w/llvm-llvm-project/clang/test/ClangScanDeps/cl-xclang.c -DPREFIX=/opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp
Exit Code: 1

Command Output (stderr):
/opt/s/w/ir/x/w/llvm-llvm-project/clang/test/ClangScanDeps/cl-xclang.c:19:11: error: CHECK: expected string not found in input
// CHECK: [[PREFIX]]/test.o:
<stdin>:1:1: note: scanning from here
pt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp/test.o: \
<stdin>:1:1: note: with "PREFIX" equal to "/opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang\\.c\\.tmp"
pt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp/test.o: \
<stdin>:1:86: note: possible intended match here
pt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp/test.o: \

Input file: <stdin>
Check file: /opt/s/w/ir/x/w/llvm-llvm-project/clang/test/ClangScanDeps/cl-xclang.c

-dump-input=help explains the following input dump.

Input was:
            1: pt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp/test.o: \ 
check:19'0     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
check:19'1                                                                                                       with "PREFIX" equal to "/opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang\\.c\\.tmp"
check:19'2                                                                                          ?            possible intended match
            2:  /opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/ClangScanDeps/Output/cl-xclang.c.tmp/test.c 
check:19'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~



Sorry for the breakage and thanks for reporting this! This is a real bug uncovered by your build using /opt. I have a fix here: D122385.

Sorry for the breakage and thanks for reporting this! This is a real bug uncovered by your build using /opt. I have a fix here: D122385.

Thanks for the fix. Could it be possible to land the fix asap?
If not, could you please revert this patch, and reland with the fix?

Thanks for the revert @gulfem. I was hoping I'll be able to quickly forward-fix the issue, but turns out there's more to investigate.

jansvoboda11 reopened this revision.Apr 11 2022, 2:03 PM
This revision is now accepted and ready to land.Apr 11 2022, 2:03 PM
jansvoboda11 planned changes to this revision.Apr 11 2022, 2:03 PM
ormris removed a subscriber: ormris.May 16 2022, 11:10 AM