This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE340530: [clangd] Suggest code-completions for overriding
base class virtual methods. (authored by kadircet, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50898?vs=162157&id=162158
hokein accepted this revision.
hokein added a comment.
still LG.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50898
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
kadircet updated this revision to Diff 162157.
kadircet added a comment.
- Resolve discussions.
- Move override generation logic from render to item generation so that
internal clients can use it as well, also use a boolean for checking override
status of a bundle to be more efficient.
- Change
kadircet updated this revision to Diff 162151.
kadircet added a comment.
- Resolve discussions.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50898
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCo
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Thanks, LGTM.
Comment at: clangd/CodeComplete.cpp:210
+const std::string Name = Method->getNameAsString();
+Overrides[Name].push_back(Method);
+ }
n
kadircet updated this revision to Diff 161963.
kadircet marked 6 inline comments as done.
kadircet added a comment.
- Resolve discussions.
- Fix a bug inside summarizeOverride.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50898
Files:
clangd/CodeComplete.cpp
clangd/CodeCo
hokein added a comment.
Looks good mostly, a few nits. We should make sure all related comments are
updated accordingly
Comment at: clangd/CodeComplete.cpp:198
+static std::vector
+getVirtualNonOverridenMethods(const DeclContext *DC, Sema *S) {
+ const auto *CR = llvm::dyn_ca
kadircet added a comment.
After today's offline discussion I suppose we are not going to implement it
within Sema. And also I think getVirtualNonOverridenMethods is a pretty generic
function that has no ties to clangd, so it can be easily moved into Sema.
Should we still move it into a separate
ioeric added a comment.
In https://reviews.llvm.org/D50898#1205756, @hokein wrote:
> I think it is reasonable to make Sema support suggesting override methods,
> instead of implementing it in clangd side?
drive-by: +1 to this.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D5
kadircet marked an inline comment as done.
kadircet added inline comments.
Comment at: clangd/CodeComplete.cpp:1233
+// struct/class/union definition.
+const auto Overrides = getVirtualNonOverridenMethods(
+Recorder->CCSema->CurContext, Recorder->CCSema);
kadircet updated this revision to Diff 161683.
kadircet marked 3 inline comments as done.
kadircet added a comment.
- Put overrides through scoring and resolve nits.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50898
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
hokein added a comment.
I think it is reasonable to make Sema support suggesting override methods,
instead of implementing it in clangd side?
Comment at: clangd/CodeComplete.cpp:206
+ llvm::StringMap> Overrides;
+ for (auto *Method : dyn_cast(DC)->methods()) {
+if (!Meth
12 matches
Mail list logo
