SVN r342929 (with tests)

This seems reasonable to me, however, I would want to give @scanon a few (work) days to see if he has any comments on the actual implementation of logb.

This looks fine to me, but this definitely should have an accompanying test.

LGTM, thanks!

It definitely shouldn't be PUBLIC, PRIVATE makes sense for this inclusion. Is there a library that pulls in the dependency (that is, do the other libraries need it due to lldbHost? If so, just adding it as INTERFACE on that library should be sufficient).

Can you use target_include_directories instead and do this only on lldbHost instead please? That restricts the inclusion to just that target, which would help prevent accidental inclusion of the headers. That is the change should be to source/Host/CMakeLists.txt.

Sure, I think I can live with this.

@t.p.northover - okay, in that case, we can leave it here. I suppose that this is caused by the pair-wise split relocations à la IMAGE_REL_ARM_MOV32T for PE/COFF?

@peter.smith, yeah, I think that sharing this might make it more complex. This looks fine to me, any other concerns?

Can you move the LOH handling into the DarwinAsmParser rather than adding the MachO specific check here please?

Ah, I see. I think I had overlooked the .incbin.

Seems fine with the adjusted comment

The install- target is created by add_llvm_install_targets, not by the component. The component is part of CMake itself, and is controlled by the CMAKE_INSTALL_DEFAULT_COMPONENT_NAME.

This seems reasonable if you need the support in the assembler. However, please add a test to ensure that the paths are mapped when invoking the assembler rather than the compiler.

Nice! I like the changes to the objdump, but, would it be possible to roundtrip the object files being added through obj2yaml and yaml2obj rather than checking in binaries?

Thanks for the changes!

It really is unfortunate that linkers can spell version differently :-(. It would be really nice to be able to unify the linker version detection.

I assume that there is/will be a different change to the frontend to enable this for users?

x86-Linux-only test is good. LG with the discussed changes.

Nice cleanup! Thanks for doing this

This makes much more sense. Thanks @joerg

This should be functionally identical and is much cleaner. Thanks!

It would be nice if we had a test case added for this, but, seems correct to me.

@rnk - Id say something more stronger than on board for this change. I think that this is a great cleanup, makes everything much clearer and generally simplifies the model that you have to work with. I didn't look at the changes to the test very carefully (they should be mechanical in nature). The rest LGTM.

SVN r333041

fix regex

SVN r331296

SVN r331282

Do you have commit access or do you need someone to commit this on your behalf?

I'm torn on this. The other -print options will perform the validation implicitly at the higher level before calling the inner functions. It is certainly reasonable to support that, but, for the common path, this check seems unnecessary (and this function is used elsewhere in clang). There is a similar problem that exists with -print-file-name=. We should probably fix both at the same time. Can you also please add a test case for this?

LGTM

Some cleanup suggestions included, but I like the change overall.

I'm not sure I understand this. The proper location for libc++ on the darwin layout is in the SDK, not relative to the driver. The default behaviour is similar to cross-compiling, and with a (derived) SDK. This definitely needs to be reviewed by @dexonsmith

Snipping bits from va_defs.h: