This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Capture the missing injected class names in findExplicitReferences.
ClosedPublic

Authored by hokein on Jan 21 2020, 2:55 AM.

Diff Detail

Event Timeline

hokein created this revision.Jan 21 2020, 2:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2020, 2:56 AM

Unit tests: pass. 62043 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

kadircet added inline comments.Jan 21 2020, 3:37 AM
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
736

looks like we are getting duplicates here

kadircet accepted this revision.Jan 21 2020, 3:43 AM

LGTM

clang-tools-extra/clangd/unittests/FindTargetTests.cpp
736

looking at this again, this is actually not caused by this change, it was always there we just didn't notice it before since no one tested class templates.

could you put a fixme on it?

This revision is now accepted and ready to land.Jan 21 2020, 3:43 AM

It would also be great to have the original rename test from the linked issue since something might potentially go wrong in-between findExplicitReferences and rename action (and also to expand the testset because the existing one is not really extensive :().

hokein updated this revision to Diff 239296.Jan 21 2020, 6:06 AM
hokein marked 2 inline comments as done.
  • add fixme
  • add rename test

It would also be great to have the original rename test from the linked issue since something might potentially go wrong in-between findExplicitReferences.

Done.

and rename action (and also to expand the testset because the existing one is not really extensive :().

yeap, there is some room to improve the test code here (e.g. using an idea similar to CodeContext in TweakTesting), previously we put the test code to a wrapper foo function, it is sufficient for most cases, but not for this patch as defining a template class in a function body is forbidden in C++. I think we can adress this afterwards.

clang-tools-extra/clangd/unittests/FindTargetTests.cpp
736

Done.

This revision was automatically updated to reflect the committed changes.

Unit tests: fail. 62042 tests passed, 1 failed and 783 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_sharedtimedmutex_requirements/thread_sharedtimedmutex_class/try_lock.pass.cpp

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml