LGTM, thanks

Nice, it looks cleaner than the override of file contents that was used before. Left one NIT comment

Left one minor suggestion , but LGTM otherwise

Thanks!

In D92355#2429117, @aaron.ballman wrote:

There's no information in either the attribute definition in Attr.td or in the documentation as to what subject this attribute applies to.

address review comments

Thanks for taking a look.

Updated for review comments

This make sense to me, thank you

Address Ahmed's comments.

LGTM, thanks for cleaning it up

This warrants a revert since it's breaking ABI compatibility for our libclang's users. @Anastasia will you be able to take a look at this soon? I plan on reverting this patch in a couple of weeks if the issue is still unresolved.

This approach seems like a reasonable compromise, thanks! LGTM.

What's wrong with using Optional<FileEntryRef> instead of MaybeFileEntryRef?

LGTM with a nit.

Don't try to fallback to the "apple-a12" cpu type when the user explicitly specifies an older cpu. The default is still "apple-a12".

Thanks. Could you expand the comment in the test that this test path specifically exercises the behavior produced by the RedirectingFileSystem?

Hmm if this unit tests doesn't reproduce it then you don't need to add it. But no further objections from me on this change.

LGTM, Thanks!

Could you add a test-case that verifies that passing -Wno-format-insufficient-args suppresses warn_printf_insufficient_data_args?

Okay, that's fair. Is there any way to trigger the computeMacroArgsCache outside of libclang, to create a clang-specific test case?

Thanks!

LGTM, with one comment below:

I think the methods that use the rewriter should be prefixed with clang_Rewriter_ to match the naming convention, e.g. clang_Rewriter_removeText, clang_Rewriter_dispose, clang_Rewriter_create.

These methods should be prefixed with clang_Cursor_ to match the naming convention.

Do you have a test case? Please add a comment stating why the check is needed.

got it, we'll do what we can to release it as soon as possible in Xcode

@jkorous if the ambiguity between char/token ranges is a problem, how about adding a getRangeCharSize that takes an unambiguous CharSourceRange range, or even making getRangeSize take the CharSourceRange instead?

Could you please add a comment stating why it's necessary to skip the module map files? Is the problem caused by the module map file that's included in the include hierarchy when it's an external module?

add missing test fix

don't check if the SDK isn't present.