Friendly ping to get this landed.

Polite ping to get this committed.

Just giving this a gentle nudge again. As I said before, there's no rush, but if there's any action I can take to get this landed, please let me know. Many thanks again, ben.

In D112222#3143966, @shafik wrote:

Apologies, this dropped off my radar it LGTM, thank you!

In D112222#3141363, @jingham wrote:

In D112222#3141355, @puremourning wrote:

Sorry, but is anything further required from me on this patch ?

You addressed an issue Shafik asked you to, so then he should make sure he's happy with the change and mark the patch accepted. OTOH we're all busy so it's easy to let this sort of thing drop. On your end, you should wait a polite interval then issue a gentle ping in his direction... A week is an okay interval, I think, so now would be appropriate.

Sorry, but is anything further required from me on this patch ?

Add some tests for dodgy inputs.
Handle invalid cases more consistently.
Factor out some copy paste code.

In D112222#3078896, @shafik wrote:

It looks good to me but can we verify it behaves nicely with undefined behavior cases e.g.:

std::string s = "Hellooooooooooooooo ";
std::string_view sv = s + "World\n";

Revert some formatting lost in rebase

Thanks for the review!

Use expect_var_path rather than just expect in test

RE-post with clang-format in path

Remove change to the formatting of empty u16 and u32 strings

I would actually prefer if we could split this into a separate change :) (Sorry!)

I'm happy to revert/split out the change for empty strings, as this is perhaps contentious (and not exactly minimal for this patch).

Erm, doh. I thought I was on main, but I was using clang 12.0.0 (tag llvmorg-12.0.0 from llvm-project). We tend to only use release tagged versions.

In D93565#2770709, @puremourning wrote:

Unless i'm missing something, this change seems to have broken scan-view, as it now just says

"ModuleNotFOundError: No module named 'Reporter'"

Due to :

import Reporter in ScanView.py

Am i losing my mind?

Unless i'm missing something, this change seems to have broken scan-view, as it now just says

In D66254#1630283, @puremourning wrote:

In D66254#1630277, @lebedev.ri wrote:

Tests?

Per my comment: I haven't added new tests to HeaderSearchTest.cc because the InMemoryFileSystem doesn't support symlinks and I didn't want to try and implement that for this patch. Let me know thoughts on alternative ways to regression test it.

In D66254#1630277, @lebedev.ri wrote:

Tests?

FYI this fixes the issue in https://github.com/clangd/clangd/issues/124

LGTM :)

In D66031#1623855, @sammccall wrote:

Thanks! Want me to land this for you?

Rebase on master

Always use physical cores rather than logical cores for best performance.

When -sync -background-index supplied, use 1 thread for background index.

In D37282#860200, @ilya-biryukov wrote:

Looks good and ready to land. Thanks for this change.
Do you have commit rights to llvm repo?

So I think this is ready now. Anything more you need from me?

Validate that the duplicate message is printed.

Tidy logging: missing newlines.

Keep ContentLength within the loop by using an inner loop to read headers.
Update tests to check stderr correctly.
Add some useful diagnostics to the log.

Thanks for the review! Few quick responses.

Couple of drive-by comments. Thanks for doing this.

I can confirm that this patch fixes a number test cases found in ycmd (a code-completion server). Specifically these ones marked as 'sic'