@dyung I cannot reproduce this issue on two different machines in both cases tests work fine. For clang and llvm-jitlink binaries (the only binaries used in the tests) my changes are non-functional and I didn't change anything in compiler-rt or ORC. Are you able to reproduce the crashes locally?

In D143436#4203008, @dyung wrote:

@DmitryPolukhin your change seems to be causing a crash in 2 ORC tests on a bot (trivial-cxa-atexit.S and trivial-atexit.S), can you take a look or revert if you need time to investigate?

@asb thank you for fixing it!

In D146156#4197289, @shafik wrote:

Is there a way to test this?

Test added

@kadircet could you please take another look?
I had to update diff to make it work when LLVM is configured without X86 targets so they are not valid and thus not injected into the command line.

Update test to use any available target

Had to revert this diff temporary due to broken ARM builds. It seems that X86 targets are disabled for that bots so tests are not passing. I'll fix upload new version for review.

Fix test on Windows

@kadircet thank you for the review! Please take another look.

Comments resolved

@kadircet it is obvious that there is something in this diff that causes this hesitancy in accepting it. I'm ready to keep iterating on the solution but I need a clue what needs the improvement. Please comment.

@kadircet and @nridge friendly ping could you please accept this diff or let me know if you would like to change anything here?
I moved addTargetAndModeForProgramName after SystemIncludeExtractor as was suggested.

In D145228#4170792, @sammccall wrote:

The install target for clang distributes the clangd static libs

I don't think this was ever intended, looks like an accidental side-effect of using LLVM's many cmake macros.
Can we fix this instead?

Rebase

@kadircet friendly ping, could you please take a look to this diff?

rebase

Rebase

Failing tests seems to be just flacky, I was not able to reprocuce any issue with them locally in stress runs and under ASan and TSan

Move call addTargetAndModeForProgramName after SystemIncludeExtractor

@nridge @kadircet please take another look, all tests now pass. As for the location of inserting driver mode please see my previous comment about MSVC CL check.

Rebase and retest, cannot reproduce test failure locally

Update test

@nridge I'm sorry, I pushed version without all tests. Now I added test case for response files, it seems that clangd hasn't had it before this patch.

Add test for expanded response files

Move standard adaptors to CommandMangler

In D143436#4122594, @kadircet wrote:

That's actually because we model compile commands pushed via LSP as a "fixed compilation database" rather than a json compilation database (you can check the code in parseFixed, near parseJson).
The reason behind the discrepancy between fixed and json compilation databases mostly arises from the fact that the former is written by people specifically to be used by clang-tooling, whereas the latter is usually provided by build system and doesn't necessarily have the same concerns as clang-tooling hence requires certain modifications.

In D143436#4120899, @nridge wrote:

Note that D138546 is removing the call to inferTargetAndDriverMode() from GlobalCompilationDatabase.cpp. It adds equivalent logic too CommandMangler which is used for CDBs pushed from LSP as well, so it accomplishes objective of this patch with respect to inferTargetAndDriverMode (but not inferMissingCompileCommands).

Fix clang-format sources

In D135367#3842950, @njames93 wrote:

LGTM, just maybe include a test case where a warning from a clang-tidy check is promoted to an error as well.

Added testcase for clang-tidy diagnostics as with --warnings-as-errors

Rebase

In D133948#3793085, @aaron.ballman wrote:

LGTM! Please add a release note for the fix when you land the changes.

clang-format

Overall looks good to me but please re-upload patch on top of some stable revision where failed test passes (it seems unrelated to your changes).

Fix clang-tidy style warning

Replace tab with spaces

In D90835#2808153, @lebedev.ri wrote:

This needs to explain why the existing functionality isn't sufficient - if the header is really not from this project,
then it should be included via -isystem, and that will naturally suppress all diagnostics from it.

Fix clang-tidy warning

@bruno and @dexonsmith thank you for the review!

Updated HeaderMapImpl::reverseLookupFilename according to the review comment

Rebase + try build on Windows again

Resolved comments

Fix forgotten comment

In D103142#2783665, @dexonsmith wrote:

Naively, this sounds like it could be a non-trivial tax on build times. But it looks like it's only called in Clang from Sema::diagnoseMissingImport, which only happens on error anyway.

Split changes into NFC https://reviews.llvm.org/D103229 and the rest

@alexfh, @njames93 and @thakis please take a look! I added all tests cases and put new logic behind a flag to make it as safe as possible.
Issue with diagnostics from macro expansion from third-party headers is the one of the biggest problem with deployment that we have and it cannot be properly fixed with wrappers around clang-tidy.

Added test for system like object macro

Fix linter

Fix clang-format issue