This looks all good to me. I'm a bit worried about the changes to AppleObjCTrampolineHandler.cpp, but from what I can see we don't actually rely on this specific field name anywhere in our code.

• Running settings export /home/teemperor/foobar1 /home/teemperor/foobar2 will actually just create foobar1, but will not report any error that foobar2 is silently ignored.

Runnings settings export crashes LLDB:

Looks good and fixes the tests for me, so I think this is good to go. Thanks for the patch!

Hmmm, maybe I mixing up something here, but the docs state pass_fds was added in 3.2:

Yeah, explicitly typing python2 is what I did to fix it. Not sure if that breaks other OSs though (e.g. if a system has no python2 binary, but only python).

AFAIR, adding an exit(...) to ConnectToRemote won't solve this problem. The test will still be failing on Arch.

Posting my mail here for the record:

Yes, the writeSocketIdToPipe(unnamed_pipe_fd, socket_id); fails in this method/file.

Sure, here is the output: After some debugging it seems that our select wrapper code is failing. We probably could fix this by terminating the process like in the other error cases in the same method (grep for the "failed to write to the unnamed pipe" error to see the code).

I'm accepting this as seems to improve the situation, even though it's not a complete fix.

FYI, for me the test is still hanging on Arch Linux.

Can't you just check for '5', as this is the only information we actually actually care about here?

LGTM minus one small detail (see inline comment). Thanks Vedant!

LGTM now, thanks for the patch!

LibCxx.h/.cpp are still not clang-formatted. I ran clang-format over the patch and pushed it here that this patch doesn't get stuck on this minor detail: https://github.com/Teemperor/lldb/commit/1c5c2f8f5d5af00420559dee52c9155e5bc72518.diff

This test is timing out for me on Arch Linux. Any idea why?

• Rebasing before merging.

• Fixed conflicts before merging.

@jingham I'm confused, what are the requested changes? Because the behavior we seem to agree on is that we keep completions with the same value but different descriptions which is already the current implementation (see lines 138 and 148 in CompletionRequestTest.cpp)

LGTM, thanks!

LGTM otherwise, but don’t know frameworks good enough to accept.

• Removed comment about redeclaring free in the test. That's wasn't correctly formulated and is anyway no longer true now that the test case including this file got bigger.
• Fixed formatting in the free declaration.

(Thanks, Richard).

@davide Maybe git is failing me, but I can only find D47508 but that doesn't seem related. Let's ping @vsk

(Phabricator didn't send my draft, sorry for the delay).

ping :)

In rL341535#287317, @xbolva00 wrote:

Can we rework GetLanguageForExpr? Or at least rename it.

LGTM from my side, but @zturner should take a look too.

(you can also try arcanist which will do this automatically for you)

Please add the header also to the modulemap in Host (which has to be manually written, as the Host module contains a lot of OS-specific headers):
module SafeMachO { header "Host/SafeMachO.h" export * }

Thanks! Didn't know we can activate this setting for the user.

This whole code doesn't seem to have test coverage, so I'll not demand a test for this. Also this patch is clearly fixing a typo, so let's get this in.

LGTM, thanks!

Don't see a way to test this, as this can only be triggered when something in LLDB is going really wrong. That piece of code also has zero test coverage, so I'll just merge it in a few weeks or so unless someone comes up with a good way to test this.

My question about a good test case template was more directed at everyone :)

• Now using a typedef instead of a macro.
• Using LLVM code style in Lazy.h
• Renamed UpdateX to CalculateX

Do we have a test case for different target byte orders that could be used as a test template here? Otherwise I would just merge this in as the previous behavior of this function was clearly wrong.

@acoomans D48465 has landed, so feel free to file bugs for any completions that are not provided by this. Thanks!