This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Add the missing elaborated types in FindTarget.
ClosedPublic

Authored by hokein on Feb 5 2020, 12:57 AM.

Diff Detail

Event Timeline

hokein created this revision.Feb 5 2020, 12:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 5 2020, 12:57 AM

Unit tests: pass. 62441 tests passed, 0 failed and 845 were skipped.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

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

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

sammccall accepted this revision.Feb 5 2020, 3:56 AM
This revision is now accepted and ready to land.Feb 5 2020, 3:56 AM
This revision was automatically updated to reflect the committed changes.
jaafar added a subscriber: jaafar.Jun 4 2020, 9:09 AM

I'm very happy this fix exists. I see it's in master, but not in 10.0.0 or 10.0.1-rc1 either. Is there any chance it can be released?

I'm very happy this fix exists. I see it's in master, but not in 10.0.0 or 10.0.1-rc1 either. Is there any chance it can be released?

unfortunately, it is not picked up in 10.0.0 release, but it is possible to get it in 10.0.1-rc1. We also have a few other crash fixes.
btw, we have weekly snapshot for clangd, https://github.com/clangd/clangd/releases, it contains latest fixes and features.

jaafar added a comment.Jun 5 2020, 9:22 AM

Thank you. I'm trying to balance stability with getting the fixes I need and so have been avoiding the "bleeding edge". I find that unfortunately the newest code tends to be "unstable", as those releases are rightfully named, and I need clangd for my daily work.

There are three commits, including this one, without which clangd cannot run for more than 10 or 20 seconds. They have all been in the codebase for several months now. I would be happy to provide a list. Thanks again.

hokein added a comment.Jun 9 2020, 6:21 AM

Thank you. I'm trying to balance stability with getting the fixes I need and so have been avoiding the "bleeding edge". I find that unfortunately the newest code tends to be "unstable", as those releases are rightfully named, and I need clangd for my daily work.

Understood.

There are three commits, including this one, without which clangd cannot run for more than 10 or 20 seconds. They have all been in the codebase for several months now. I would be happy to provide a list. Thanks again.

I will come up with a list, it would be nice if you can provide a list of commits, to make sure nothing left out.

jaafar added a comment.Jun 9 2020, 5:29 PM

Thnaks very much @hokein . The ones I need are:
9a5c448a31bacc08e73fcae4636094f9b6e2be6a
eaf0c89ec5f866b6cef296c542c030bb2cf8481d
d66afd6dde542dc373f87e07fe764c071fe20d76
as well as this patch, under review but dormant since February: https://reviews.llvm.org/D74731

With these fixes I have a pleasant experience on my large codebases. Thanks for your help.

@jaafar Thanks for the list, this is really useful for us. These all look reasonable to me, I've filed bugs to track them and if they look good to Haojian too we'll get them merged. See https://github.com/clangd/clangd/issues?q=is%3Aissue+label%3Acherrypick

I've added an issue template to the clangd bugtracker, in case you find more such things in the future: https://github.com/clangd/clangd/issues/new/choose.

Regarding the stalled D74731, I'll try to get some version of that committed so we can cherrypick it too.

These changes look reasonable to me, thanks!
Here is a list of commits that we plan to cherrypick, see https://github.com/clangd/clangd/issues/417.