jubnzv added inline comments.
================ Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp:7 + +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t + ---------------- dyung wrote: > jubnzv wrote: > > probinson wrote: > > > probinson wrote: > > > > TWeaver wrote: > > > > > njames93 wrote: > > > > > > TWeaver wrote: > > > > > > > Is the missing FileCheck call here on purpose? it seems to me > > > > > > > that the CHECK-MESSAGES aren't actually being verified by this > > > > > > > test? > > > > > > > > > > > > > > unless I'm missing something. > > > > > > > > > > > > > > TIA > > > > > > `check_clang_tidy` invokes FileCheck. Does something else make you > > > > > > think these labels are being tested?? > > > > > whilst investigating an unrelated issue on our internal branch, I > > > > > tried editting the check lines in this test and wasn't able to create > > > > > a failure. but if I add > > > > > > > > > > '| FileCheck %s -check-prefix=CHECK-MESSAGES' to the run line and > > > > > then edit the checks, I can induce an error. > > > > > > > > > > This could be an issue on our internal branch though... :shrug: > > > > > thanks for the speedy reply. > > > > I'm suspicious that our downstream problem is because the test is > > > > assuming that the target is Windows, just because the host is. That's > > > > not true for us (or anyone with a Windows-hosted cross-compiler). Does > > > > clang-tidy accept a target triple? > > > Or possibly the test could be set up to require a Windows target, rather > > > than a Windows host. > > I'm not sure that there is a way to pass a target triple to `clang-tidy`. > > > > But `llvm-lit` should not run this test on non-Windows host because of > > `REQUIRES: system-windows`. For example, when I trying to run it on my > > Debian machine (`llvm-lit > > clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp > > -v`) I got the following result: > > > > ``` > > -- Testing: 1 tests, 1 workers -- > > UNSUPPORTED: Clang Tools :: > > clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp (1 of 1) > > > > Testing Time: 0.01s > > Unsupported: 1 > > ``` > Our problem is the opposite, we are running a cross compiler hosted on > Windows, but targeting a non-Windows platform (PS4 in our case). Well, maybe it's a good idea to add the option to set target triple to //clang-tidy//? I think this makes sense because //clang-tidy// can be used to check sources for a project built using a cross-toolchain. In the case, when the platform is set incorrectly, the tool can works incorrectly, because now we have platform-dependent checks. This also will also help to get rid of similar problems if other platform-dependent tests are added. I will prepare a patch that adds the `-target` option within a few days. If anyone has any other ideas on how to work around this problem, I'm open to suggestions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101259/new/ https://reviews.llvm.org/D101259 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
