Page MenuHomePhabricator

[Index] Set OrigD before D is changed.
ClosedPublic

Authored by ioeric on Jul 18 2018, 5:13 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

ioeric created this revision.Jul 18 2018, 5:13 AM

Is it possible to add a regression test case ? I assume this is fixing some issue, we should make sure we don't regress again.

Is it possible to add a regression test case ? I assume this is fixing some issue, we should make sure we don't regress again.

This fixes a downstream use case where we use OrigD. AFAICT, c-index-test is used to tests the index library, but it doesn't seem to use OrigD at all. We could probably add a flag to c-index-test to optionally print out OrigD. I didn't go down this path because I thought this change is trivial and doesn't break any existing changes. If you think optionally checking OrigD is reasonable, I'd be happy to add that with a test case.

CXIndexDataConsumer.cpp uses ASTNode.OrigD, that code is exercised when you run c-index-test -index-file <...>. I'd recommend to at least check if that command would produce a different output for the test case that you detected originally.

ioeric updated this revision to Diff 156356.Jul 19 2018, 2:17 PM
  • try to add a test case.

CXIndexDataConsumer.cpp uses ASTNode.OrigD, that code is exercised when you run c-index-test -index-file <...>. I'd recommend to at least check if that command would produce a different output for the test case that you detected originally.

Thanks for the pointer!

I've given this a try but still didn't manage to come up with a test that would produce different outputs. Basically, what I'm trying to achieve is to get the original (implicit) template instantiation declarations from their references (see the added test case). This is hard to verify with c-index-test because 1) for declarations, although OrigD is used in the test, implicit instantiations are simply discarded (https://github.com/llvm-mirror/clang/blob/master/lib/Index/IndexingContext.cpp#L354) and 2) for references, c-index-test doesn't use OrigD to print symbol info.

Let me know if there is a better approach.

akyrtzi accepted this revision.Jul 19 2018, 2:55 PM

Good enough, thanks for looking into this!

This revision is now accepted and ready to land.Jul 19 2018, 2:55 PM
This revision was automatically updated to reflect the committed changes.