To clarify, I was suggesting that in addition to removing the space from unhandled types, we also handle the member function pointer type and not leave it in this fallback case.
Types should have unique USR characters so that overloaded functions (overloaded on the type parameter) have unique USRs.

Maybe we could also handle this kind of type instead of leaving it 'unhandled'? What Type is it?

In D97204#2586111, @rsmith wrote:

Can we avoid a libclang ABI break if we don't allow the use of 64-bit source locations for builds with 32-bit pointers?

A major thing worth noting is that 64-bit source locations will
require an ABI breakage in libclang. This patch changes the bit width
in libclang unconditionally, rather than making it configurable.

clang-format change

Fix typo in commit message, 'state' -> 'stale'

Use getValueOr

clang-format changes

I'd like if we only had to use one flag (-fallow-pcm-with-compiler-errors) and have it handle both modules and PCH. Could you make the flag also work for PCH and/or add a test that verifies it works?
You may only have to change

Opts.AllowPCHWithCompilerErrors = Args.hasArg(OPT_fallow_pch_with_errors);

to

Opts.AllowPCHWithCompilerErrors = Args.hasArg(OPT_fallow_pch_with_errors, OPT_fallow_pcm_with_errors);

Thank you! Are you able to commit it by yourself?

Good catch!

In D86990#2252592, @jkorous wrote:

@akyrtzi IIUC you are saying we should try to push the char-range semantics from libclang API as deep as possible to see if we could avoid this annoying conversion. I think that's a reasonable long-term strategy.
But I also think we'll inevitably run into some clang APIs that use SourceRange for valid reasons and we won't be able to change those. Some other clang APIs might require significant amount of work to be enhanced.

Ultimately, given this conversion isn't part of public libclang API and has explicit semantics, I would keep it - I'd just add a warning to its doc comment.

On practical level - I need this conversion for my other work and I can't go just now and refactor the Rewriter interface.
https://reviews.llvm.org/D86992

SourceRange cxloc::translateCharRangeToTokenRange(CXTranslationUnit TU, CXSourceRange R) is an expensive operation and I'm concerned it will be easy to start calling at places and introduce performance degradation.

We may have places in the code where SourceRange is used as a pair of locations, and those locations are character locations instead of token ones, so essentially the information of whether the range is token-based or character-based gets lost, and we get into trouble when passing such a SourceRange to APIs that assume token-based.

In D86840#2248463, @jkorous wrote:

Thanks a lot for this clarification! So, does that mean the issue is actually here?

static inline SourceRange translateCXSourceRange(CXSourceRange R) {
  return SourceRange(SourceLocation::getFromRawEncoding(R.begin_int_data),
                     SourceLocation::getFromRawEncoding(R.end_int_data));
}

AFAIK SourceRange is supposed to always represent a token range (begin loc points to beginning of first token, end loc points to beginning of last token). For representing a character range, CharSourceRange should be used, though IMO its IsTokenRange member was a mistake, CharSourceRange should have only being used to represent half-open character-based range. This distinction is thankfully more clear on the Swift side.

I agree it makes sense to disable CLANG_ENABLE_ARCMT if CLANG_ENABLE_STATIC_ANALYZER is also disabled.