This is an archive of the discontinued LLVM Phabricator instance.

[clang] Apply FixIts to members declared via `using` in derived classes
ClosedPublic

Authored by denis-fatkulin on Aug 3 2022, 12:05 PM.

Details

Summary

FixIt don't switch to arrow in derrived members with using

Example code:

struct Bar {
  void foo();
};
struct Baz {
  using Bar::foo;
};
void test(Baz* ptr) {
  ptr.^
}

Diff Detail

Event Timeline

denis-fatkulin created this revision.Aug 3 2022, 12:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2022, 12:05 PM
denis-fatkulin requested review of this revision.Aug 3 2022, 12:05 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 3 2022, 12:05 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
kbobyrev edited reviewers, added: kadircet, sammccall; removed: kbobyrev.Aug 3 2022, 3:20 PM
kadircet accepted this revision.Aug 8 2022, 9:21 AM

thanks lgtm!

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
2187 ↗(On Diff #449733)

no need for this test in clangd, as it's testing the same thing you've in clang lit test (and clang is the righteous place to test this behavior)

clang/lib/Sema/SemaCodeComplete.cpp
1101–1102

can you also fix the constructor call here?

1277–1278

you can just pass std::move(R.FixIts) as the next argument in this constructor call instead.

This revision is now accepted and ready to land.Aug 8 2022, 9:21 AM

Patch is updatetd according to remarks by @kadircet.

@kadircet, could you please also merge my pacth to code base? I haven't such permissions yet.
My git user name and address: Denis Fatkulin (fatkulin.denis@huawei.com)

Thank you for the review!

denis-fatkulin marked an inline comment as done.Aug 8 2022, 1:52 PM
denis-fatkulin added inline comments.
clang/lib/Sema/SemaCodeComplete.cpp
1277–1278

Fixed.

denis-fatkulin marked 3 inline comments as done.Aug 8 2022, 1:54 PM

looks like you've uploaded the diff without context, can you upload it again with full context? also the changes to MaybeAddResult seem to be missing.

Patch is updated with context