This revision was automatically updated to reflect the committed changes.
Closed by commit rGa76e68c9704f: [CodeComplete] Member completion for
concept-constrained types. (authored by sammccall).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
sammccall marked an inline comment as done.
sammccall added inline comments.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4984
+if (Q && isApprox(Q->getAsType(), T))
+ addType(NNS->getAsIdentifier());
+ }
kadircet wrote:
> sammccall
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
LGTM. thanks!
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4984
+if (Q && isApprox(Q->getAsType(), T))
+ addType(NNS->getAsIdentifier());
+ }
sammccall added a comment.
Thanks!
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4905
+ // A requires(){...} lets us infer members from each requirement.
+ for (const concepts::Requirement *Req : RE->getRequirements()) {
+if (!Req->isDependent())
sammccall updated this revision to Diff 252917.
sammccall marked 13 inline comments as done.
sammccall added a comment.
address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
Files:
kadircet added inline comments.
Comment at: clang/include/clang/Sema/Scope.h:323
/// declared in.
- bool isDeclScope(Decl *D) {
-return DeclsInScope.count(D) != 0;
- }
+ bool isDeclScope(const Decl *D) { return DeclsInScope.count(D) != 0; }
also mark
sammccall added a comment.
@kadircet I realized this change was gathering dust, and you've touched
SemaCodeComplete in recent memory... any interest?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
nridge added a comment.
Thanks! Consider the patch "accepted" from my POV.
One last nit: please link to https://github.com/clangd/clangd/issues/261 in the
commit message
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
sammccall updated this revision to Diff 247993.
sammccall added a comment.
comment and tweak order of AccessOperator
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
Files:
sammccall marked an inline comment as done.
sammccall added inline comments.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5006
+ if (/*Inserted*/ R.second ||
+ std::make_tuple(M.Operator, M.ArgTypes.hasValue(),
+ M.ResultType != nullptr)
nridge added inline comments.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4972
+
+// In T::foo::bar, `foo` must be a type.
+bool VisitNestedNameSpecifier(NestedNameSpecifier *NNS) {
nridge wrote:
> sammccall wrote:
> > nridge wrote:
> > > It would be
nridge marked 3 inline comments as done.
nridge added inline comments.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4856
+private:
+ // Infer members of T, given that the expression E (dependent on T) is true.
+ void believe(const Expr *E, const TemplateTypeParmType *T) {
sammccall added a comment.
> I'm not sure that I'm qualified to approve this patch (this is my first time
> looking at clang's completion code)
I feel the same way about writing the code :-)
Thanks for the comments! I'll get another review from someone who's stared into
this abyss before, too.
sammccall updated this revision to Diff 247751.
sammccall marked 27 inline comments as done.
sammccall added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
Files:
njames93 added inline comments.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4860
+ return;
+if (auto *CSE = llvm::dyn_cast(E)) {
+ // If the concept is
nridge wrote:
> clang-tidy gives me an `'auto *CSE' can be declared as 'const auto *CSE'`
>
nridge added a comment.
I'm not sure that I'm qualified to approve this patch (this is my first time
looking at clang's completion code), but I did look through the entire patch
now, and it looks good to me modulo the mentioned, mostly minor, comments.
Comment at:
sammccall marked 3 inline comments as done.
sammccall added inline comments.
Comment at: clang/lib/Sema/CodeCompleteConsumer.cpp:592
}
+for (const FixItHint : Results[I].FixIts) {
+ const SourceLocation BLoc = FixIt.RemoveRange.getBegin();
(This
nridge added a comment.
Thanks for looking at this! Sorry for the late response, I was travelling for a
few weeks.
So far I've only had a chance to look at the tests.
Comment at: clang/test/CodeCompletion/concepts.cpp:34
+ // RUN: | FileCheck %s -check-prefix=DOT
merge_guards_bot added a comment.
{icon check-circle color=green} Unit tests: pass. 62282 tests passed, 0 failed
and 827 were skipped.
{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 7
warnings
sammccall created this revision.
sammccall added reviewers: nridge, saar.raz.
Herald added subscribers: cfe-commits, mgrang.
Herald added a project: clang.
The basic idea is to walk through the concept definition, looking for
t.foo() where t has the constrained type.
In this patch:
- nested
20 matches
Mail list logo
