This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Handle DeducedTemplateSpecializationType in TargetFinder
ClosedPublic

Authored by nridge on Jan 2 2020, 5:15 PM.

Details

Summary

This is mostly a workaround for
https://bugs.llvm.org/show_bug.cgi?id=42914. Once that is fixed, the
handling in VisitDeducedTyped() should be sufficient.

Fixes https://github.com/clangd/clangd/issues/242

Diff Detail

Event Timeline

nridge created this revision.Jan 2 2020, 5:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 2 2020, 5:15 PM

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

kadircet added inline comments.
clang-tools-extra/clangd/unittests/XRefsTests.cpp
499 ↗(On Diff #235975)

could you rather add this test into FindTargetTests.cpp ?

ilya-biryukov requested changes to this revision.Jan 3 2020, 1:01 AM

Please add a test for findExplicitReferences too

This revision now requires changes to proceed.Jan 3 2020, 1:01 AM

Thanks!

clang-tools-extra/clangd/FindTarget.cpp
361

This needs a comment referencing the bug, and describing the consequence (we're going to miss the instantiation even when it's in principle known)

nridge updated this revision to Diff 236652.Jan 7 2020, 11:45 AM
nridge marked an inline comment as done.

Address review comments

nridge updated this revision to Diff 236653.Jan 7 2020, 11:47 AM

Remove no longer used flag from XRefTests.cpp

Unit tests: pass. 61301 tests passed, 0 failed and 736 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

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

Unit tests: pass. 61301 tests passed, 0 failed and 736 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: pass.

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

ilya-biryukov accepted this revision.Jan 7 2020, 11:09 PM

LGTM

clang-tools-extra/clangd/FindTarget.cpp
370

NIT: remove braces

This revision is now accepted and ready to land.Jan 7 2020, 11:09 PM
ilya-biryukov added a comment.EditedJan 7 2020, 11:15 PM

I'm also looking into fixing this in clang, this shouldn't be too hard.
But please land the workaround for now.

Here's my attempt at storing the deduced type in TypeLocs: D72442.
It almost worked, but still needs an update to fix some test failures.

The change proves it's possible to do this in principle, although it does require a few hacky tweaks to type and decl printing to make sure we produce the same output in diagnostics and other places that need it.

This revision was automatically updated to reflect the committed changes.