Page MenuHomePhabricator

[clangd] Support renaming designated initializers
ClosedPublic

Authored by kbobyrev on Jan 16 2020, 12:51 PM.

Details

Summary

Clangd does not find references of designated iniitializers yet and, as a
result, is unable to rename such references. This patch addresses this issue.

Resolves: https://github.com/clangd/clangd/issues/247

Diff Detail

Event Timeline

kbobyrev created this revision.Jan 16 2020, 12:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 16 2020, 12:51 PM
kbobyrev updated this revision to Diff 238586.Jan 16 2020, 12:52 PM

Remove duplicated testt

Unit tests: fail. 61928 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

Unit tests: pass. 61929 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

Harbormaster completed remote builds in B44189: Diff 238585.

Unit tests: pass. 61929 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

sammccall added inline comments.Jan 21 2020, 2:34 AM
clang-tools-extra/clangd/FindTarget.cpp
669

you're breaking after the first one - I think you'd like to report every one instead?
You'd test this with a DIE like { .Foo.Bar = 2 } where Foo has struct type.

(targetDecl only reports the *last* one, because the Designator can't be a DynTypedNode, but we don't care about that here)

clang-tools-extra/clangd/unittests/FindTargetTests.cpp
599–600

isn't this spelled c++20 yet?

sammccall marked an inline comment as done.Jan 22 2020, 2:08 AM
sammccall added inline comments.
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
599–600

(No, it's not)

kbobyrev planned changes to this revision.Jan 23 2020, 4:45 PM
kbobyrev marked an inline comment as done.

Going to deal with nested designated inits soon and update the patch.

kbobyrev updated this revision to Diff 242040.Mon, Feb 3, 5:18 AM
kbobyrev marked an inline comment as done.

Add tests for nested designated initializers.

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

Sorry, I tried to understand this comment but I wasn't able to in the end. Could you please elaborate on this?

I'm adding the test cases that I originally thought you meant, but those work correctly so I assume I didn't understand what case you were referring to.

Unit tests: pass. 62405 tests passed, 0 failed and 839 were skipped.

clang-tidy: pass.

clang-format: pass.

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.Mon, Feb 10, 2:32 AM
sammccall added inline comments.
clang-tools-extra/clangd/FindTarget.cpp
669

Sorry, I'm not sure what I was thinking here - code LG.

This revision is now accepted and ready to land.Mon, Feb 10, 2:32 AM
This revision was automatically updated to reflect the committed changes.