@labath Thanks for the hint with the sysroot, but I'm not really sure what's the best way to test this. Making a whole fake sysroot that can be used to compile a std module seems overkill, and not sure if symlinking or so is a good approach either. I'm open to suggestions.

• Rebase on top of Adrian's patch (Thanks!)
• Addressed the feedback from Shafik/Pavel.

• Added #ifdef for 3.8 release.

landed in llvm-svn: 354043

We could also check the libc++ version via _LIBCPP_VERSION and just keep the old code around behind an ifdef?

Ah yes, that also explains why the test suit failed. Fixed and the test now passes, thanks!

LGTM.

+1. That's a cleaner version of what I had to do in D58125, so feel free to commit when done. The code is similar enough that rebasing my patch on top shouldn't take too much time.

• Cleaned up setup method.

All the changes regarding extracting module include paths are probably superseded by Adrian's patch (which does the same thing nicer). So all the code related to GetModuleIncludes will be removed from this patch most likely.

@JDevlieghere I think this is the LLVM homepage (not LLDB), so I believe there is no RST file to edit?

Just trying to get some early feedback before I subscribe the mailing list. Also let me know if someone here also wants to mentor for this project and I'll add you.

LGTM for the CloneChecker changes.

LGTM, thanks for the patch!

LGTM, thanks!

• Added missing include.

So, the idea of going back to the headers and see if we can potentially remove mm_malloc from the modulemap didn't work out (mostly because a lot of headers include it indirectly).

• Moved define to Xcode's Config.h (Thanks Pavel!)

I currently don't have access to a macOS machine and this only affects macOS, so I would appreciate if someone could check if this still compiles/works as intended. Otherwise I can also just commit it and watch the macOS bots.

• Added a comment that the using directives are created by Sema.

• Moved code into the VisitFunctionDecl function.

+1. Especially since our assumption that everything related to C++ is of non-zero size is also not entirely correct. Empty structs for example are actually size 0 for Clang until we reach code gen where we set their size to 1. That's why we have this bug where we can't call functions that return an empty struct.

Didn't really upstream any non-trivial ASTImporter patches yet, so please point out any style errors.

@joerg Yeah, we saw the commit explaining why the original fwd declaration patch was reverted. However, from what I can see, we only have three ways to fix the cyclic dependency between glibc and Clang's internal module:

This looks all good to me and from what see this was requested in a previous review (D44188). Do you need someone to commit this or did you receive commit access?

LGTM modulo the unrelated clang-format changes. Feel free to commit them separately (even though the m_event_names fix looks a bit strange).

Do we also want to get rid of the // C Includes comments that are in some files? LLVM isn't using them either and just removing lines doesn't make git blame more complicated.

LGTM, sorry for that!

Yeah it uses the color settings of the diagnostic engine which are probably set to false in LLDB. I think activating colors there should fix the issue.

I think Pavel's point is to call the dump overload which allows specifying our own custom raw_ostream: https://clang.llvm.org/doxygen/classclang_1_1Decl.html#a278b3b87b6f9d3b20ed566a8684341a6 And our raw_ostream is configured by LLDB to correctly choose if we want color or not.

Minor detail: The revision title only mentions Java, but this revision removes both Go and Java.

This looks nice, just added some minor comments below. Otherwise LGTM after Davide's point is addressed.

Woops, I wanted to accept in my previous comment.