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.

Are you sure the difference is not random noise? It's not clear to me under which conditions you would encounter the same region in NewParsedRegions within the same TU. I assume this would be the case if the same file is parsed multiple times, but I'd expect this to be uncommon within the same TU.
I'd recommend to add some counters of when NewParsedRegions.count(region) is true and when ParsedRegions.count(region) is true, and print them at the end.

In D65846#1619752, @bruno wrote:

clang -fmodules -fmodules-cache-path=... is supposed to create the directory for the cache path, including the parent directories, AFAIK. If this doesn't happen it is a behavior change (and undesirable IMO).

Is c-index-test invoking clang or do we just have a similar interface? Perhaps it's not doing it right (haven't seen this problem happening directly while invoking clang). Should -fallow-pch-with-compiler-errors be considered somehow here?

In D65846#1619645, @jfb wrote:

My current guess is that this part of the test:

c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp -Xclang -triple -Xclang x86_64-apple-darwin

Is expected to generate the unknown type name error, but when than happens it ignores -fmodules-cache-path=%t.mcp and doesn't create the directory. The next line expects the directory to exist, which is why it can't create the lock file (because the directory it's trying to create it in doesn't exist).

Sorry, it's not clear to me what is the issue.

In D64384#1596521, @jkorous wrote:

For now I went with IndexWhileBuilding name but I'm open to discussion.