The newly added test is flaky and it fails on the Windows bot every few runs. I also saw it fail on Ubuntu in our private buildbots.
Failing build:
https://lab.llvm.org/buildbot/#/builders/13/builds/8922

In D101128#2831098, @wallace wrote:

I'm reverting it asap

@wallace : Your most recent change to comment out ProgressEventThreadFunction is causing warnings to be generated when building with clang. Please have a look:

In D88666#2828306, @thakis wrote:

We also see check-all timeout recently (fairly consistently), see https://bugs.chromium.org/p/chromium/issues/detail?id=1221702

Since Stella reported problems with this too, I speculatively reverted it (and follow-ups) in fb32de9e97af0921242a021e30020ffacf7aa6e2 for now.

I wasn't able to reproduce this locally by running *just* the DirectoryWatcher tests. I'm not running all of the clang tests repeatedly to see if I can get a repro that way.

This is the best I can do from the online builds. I'll try and repro locally as well:

In D88666#2825300, @compnerd wrote:

Interesting, are the logs from the runs available? I have run the test ~10000 times locally and its been stable. Perhaps the logs can show what is going on.

We still occasionally (every couple of runs) see these tests hang on Windows in both Debug and Release. Unfortunately, I don't have access to the machines running the tests to debug the tests while they are hanging and I haven't had a chance to try to reproduce locally.

In the last week or so I've seen one of these tests fail occasionally with ACCESS_VIOLATION in our testing. Unfortunately, I can't get a local repro. Here's the (best) log I can get from our online testing:

The Windows buildbot does not like signposts:

This change is generating warnings when building with clang:

It looks like this caused a couple of failures on the Windows LLDB bot due to crashes in clang:

In D103869#2806893, @stella.stamenova wrote:

It looks like this is causing a build failure on the windows mlir buildbot which may be the goal since the assert is firing, but it would be nice to keep the buildbot green.

https://lab.llvm.org/buildbot/#/builders/13/builds/8726

FAILED: tools/mlir/lib/IR/CMakeFiles/obj.MLIRIR.dir/BuiltinDialect.cpp.obj 
C:\PROGRA~2\MICROS~3\2017\COMMUN~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\cl.exe  /nologo /TP -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=1 -DMLIR_ROCM_CONVERSIONS_ENABLED=1 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\mlir\lib\IR -IC:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\lib\IR -Iinclude -IC:\buildbot\mlir-x64-windows-ninja\llvm-project\llvm\include -IC:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\include -Itools\mlir\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:__cplusplus /Zc:strictStrings /Oi /Zc:rvalueCast /bigobj /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /Gw /MD /O2 /Ob2  /EHs-c- /GR- -UNDEBUG -std:c++14 /showIncludes /Fotools\mlir\lib\IR\CMakeFiles\obj.MLIRIR.dir\BuiltinDialect.cpp.obj /Fdtools\mlir\lib\IR\CMakeFiles\obj.MLIRIR.dir\ /FS -c C:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\lib\IR\BuiltinDialect.cpp
C:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\include\mlir/IR/OpDefinition.h(1783): error C2338: Op class aren't allowed to have data members
C:\buildbot\mlir-x64-windows-ninja\llvm-project\mlir\include\mlir/IR/OpDefinition.h(1782): note: while compiling class template member function 'mlir::LogicalResult mlir::Op<mlir::UnrealizedConversionCastOp,mlir::OpTrait::ZeroRegion,mlir::OpTrait::VariadicResults,mlir::OpTrait::ZeroSuccessor,mlir::OpTrait::VariadicOperands,mlir::CastOpInterface::Trait,mlir::MemoryEffectOpInterface::Trait>::verifyInvariants(mlir::Operation *)'

It looks like this is causing a build failure on the windows mlir buildbot which may be the goal since the assert is firing, but it would be nice to keep the buildbot green.

In D103394#2794112, @mehdi_amini wrote:

(fixed the warning in 8c948b18e9d8 by the way)

In D103394#2794104, @mehdi_amini wrote:

In D103394#2793950, @stella.stamenova wrote:

Changing the return type of a function is not generally considered a NFC.

It is from the point of view of the compiler: it won't affect its behavior in any way.
A good litmus test is: "do we need a test for this patch?" ; if not then it is likely NFC.

I would not consider this an NFC and I think the title should be updated to not include the NFC designation. It might not be changing the intended functionality, but it is changing the logic (and functions called) to achieve the result.

This change is causing the following warning when building with clang:

I'm not sure what to make of it now, and I'm not particularly familiar with Windows or MSVC. I'm still amenable to changing the code, but I also fear the changes I made won't actually fix the issue as I am only testing on godbolt. Is there any shortcut to testing with the Windows buildbots? Or do I have to install Windows somewhere and try to reproduce it?

@dblaikie @scott.linder I updated VS on both the mlir and lldb windows buildbots (from 15.9.30 to 15.9.36), but this update does not have an update to the compiler and indeed the compiler version was already `Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27045```. You can see from the logs of the broken builds as well:

In D100672#2767612, @dblaikie wrote:

In D100672#2766442, @scott.linder wrote:

This seems like a bug in parameter pack expansion in older versions of MSVC, compare 19.10 https://godbolt.org/z/TMd3K8WoK with 19.14 https://godbolt.org/z/9c9TT8bnb

@stella.stamenova is there any possibility the build bots could be updated to the latest MSVC 2017 update? https://llvm.org/docs/GettingStartedVS.html#software says "Visual Studio 2017 or higher, with the latest Update installed", but I could also see us wanting to support any VS 2017 release if possible.

If we want to support all updates of VS 2017 I can just substitute in the old definition of llvm::TypeIndex in place of the new llvm::FirstIndexOfType, but I might have to think more about how to work around this in llvm::TypesAreDistinct

I'd generally be happy to stick with the original requirement. @stella.stamenova - any chance you could update the bot(s) here?

This broke some windows bots:

I don't have context into this either. Does this cause any test failures or is everything still green after the change?

LGTM.

It looks like this broke the Windows LLDB bot: https://lab.llvm.org/buildbot/#/builders/83/builds/6085

Looks like this is failing on the Windows LLDB bot:

This broke several windows bots, for example the MLIR one:

This broke the windows LLDB bot. Please fix it soon or revert.

It looks like this broke the Windows LLDB bot (which also builds lld):

It looks like @teemperor fixed it already!

The latest change (commit be0ced03) issues the following warning:

Looks like this broke the Windows lldb buildbot:

Sounds like a plan. I'm not looking forward to having to undo and then redo all the necessary changes on our end, but we will have the opportunity to do them better with a little more heads up next time.

In D95713#2647548, @teemperor wrote:

The tests are failing because Dave's bot is running without enabled Python. The same is true for the Windows bot. Putting the plugin behind #ifdef LLDB_ENABLE_PYTHON should fix this.

This switch to top down traversal also impacts onnx-mlir which has passes that do not currently work correctly with top down traversal (but do work with bottom up). I realize it's hard to keep track of all possible users, but I think it makes a lot of sense to make such changes optional for a time until the downstream components can adjust without breaking.

This did end up breaking the windows build bot...

In D98842#2634902, @teemperor wrote:

Just gonna post a comment to point out that this apparently adds all tests twice to check-all which makes the tests go kaputt (but I only messaged Jonas directly about that, hence the lack of detailed errors here).

Looks good as long as it doesn't break windows :O

This broke the windows bot:

It looks like this is failing on Windows MLIR bot:

In D95711#2595566, @mib wrote:

In D95711#2595458, @stella.stamenova wrote:

This does not build on the Windows bot:

https://lab.llvm.org/buildbot/#/builders/83/builds/4214

Should be fixed with 5a9c34918bb1526b7e8c29aa5e4fb8d8e27e27b4 ... I'm also investigating some test failures that are happening on Linux, that will also probably happen on Windows. I should have a fix for that soon.

In D97584#2592467, @jdoerfert wrote:

LG, could you check if this is necessary after D97238 ?

This does not build on the Windows bot:

In D96778#2595162, @jankratochvil wrote:

In D96778#2595110, @stella.stamenova wrote:

What is the actual test requirement?

It requires x86_64 (it is incompatible with x86_32) and hopefully it is OS-agnostic (after fixing the %clang_host).

Considering this broke two bots, you might want to commit a fix or revert soon.

In D96778#2594405, @jankratochvil wrote:

@stella.stamenova It failed on a Windows box:
https://lab.llvm.org/buildbot/#/builders/83/builds/4202

$ "c:\buildbot\lldb-x64-windows-ninja\build\bin\clang.exe" "--target=specify-a-target-or-use-a-_host-substitution" "-o" "C:\buildbot\lldb-x64-windows-ninja\build\tools\lldb\test\SymbolFile\DWARF\Output\DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s.tmp" "--target=x86_64-pc-linux" "C:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\test\Shell\SymbolFile\DWARF\DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu1.s" "C:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\test\Shell\SymbolFile\DWARF/Inputs/DW_TAG_variable-DW_AT_decl_file-DW_AT_abstract_origin-crosscu2.s"
# command stderr:
clang: error: unable to execute command: program not executable

Do you think the following change would fix it?

-// RUN: %clang -o %t --target=x86_64-pc-linux %s \
+// RUN: %clang_host -o %t %s \

LGTM.

It looks like this change resulted in a failing test on the Windows LLDB bot:

To be clear, the windows mlir buildbot is broken because of this change:

It looks like this broke the windows mlir bot. Please revert it or fix it ASAP:

This worked almost, but not quite. It seems you did not fully update RunInTerminal.cpp.