[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
Files:
clang/include/clang/Sema/Scope.h
clang/lib/Sema/CodeCompleteConsumer.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/test/CodeCompletion/concepts.cpp
Index: clang/test/CodeCompletion/concepts.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/concepts.cpp
@@ -0,0 +1,59 @@
+template concept convertible_to = true;
+template concept same_as = true;
+template concept integral = true;
+
+template
+concept W = requires(A a, B b) {
+ { b.www } noexcept -> integral;
+};
+
+template concept X = requires(T t) {
+ t.xxx(42);
+ typename T::xxx_t;
+ T::xyz::member;
+};
+
+template
+concept Y = requires(T t, U u) { t.yyy(u); };
+
+template
+concept Z = requires(T t) {
+ { t.zzz() } -> same_as;
+ requires W;
+};
+
+// Concept constraints in all three slots require X, Y, Z, and ad-hoc stuff.
+template
+requires Y && requires(T *t) { { t->aaa() } -> convertible_to; }
+void foo(T t) requires Z || requires(T ) { t.bbb(); t->bb(); } {
+ t.x;
+ t->x;
+ T::x;
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:29:5 %s \
+ // RUN: | FileCheck %s -check-prefix=DOT -implicit-check-not=xxx_t
+ // DOT: Pattern : [#convertible_to#]aaa()
+ // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
+ // DOT: Pattern : bbb()
+ // DOT: Pattern : [#integral#]www
+ // DOT: Pattern : xxx(<#int#>)
+ // FIXME: it would be nice to have int instead of U here.
+ // DOT: Pattern : yyy(<#U#>)
+ // DOT: Pattern : [#int#]zzz()
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:30:6 %s \
+ // RUN: | FileCheck %s -check-prefix=ARROW -implicit-check-not=xxx_t
+ // ARROW: Pattern : [#convertible_to#]aaa() (requires fix-it: {{.*}} to ".")
+ // ARROW: Pattern : bb()
+ // ARROW: Pattern : bbb() (requires fix-it
+ // ARROW: Pattern : [#integral#]www (requires fix-it
+ // ARROW: Pattern : xxx(<#int#>) (requires fix-it
+ // ARROW: Pattern : yyy(<#U#>) (requires fix-it
+ // ARROW: Pattern : [#int#]zzz() (requires fix-it
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:31:6 %s \
+ // RUN: | FileCheck %s -check-prefix=COLONS -implicit-check-not=yyy
+ // COLONS: Pattern : xxx_t
+ // COLONS: Pattern : xyz
+}
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9,6 +9,7 @@
// This file defines the code-completion semantic actions.
//
//===--===//
+#include "clang/AST/ASTConcept.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
@@ -16,8 +17,11 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprConcepts.h"
#include "clang/AST/ExprObjC.h"
+#include "clang/AST/NestedNameSpecifier.h"
#include "clang/AST/QualTypeNames.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/Type.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/Specifiers.h"
@@ -4746,6 +4750,369 @@
return nullptr;
}
+namespace {
+// Collects completion-relevant information about a concept-constrainted type T.
+// In particular, examines the constraint expressions to find members of T.
+//
+// The design is very simple: we walk down each constraint looking for
+// expressions of the form T.foo().
+// If we're extra lucky, the return type is specified.
+// We don't do any clever handling of && or || in constraint expressions, we
+// take members from both branches.
+//
+// For example, given:
+// template concept X = requires (T t, string& s) { t.print(s); };
+// template void foo(U u) { u.^ }
+// We want to suggest the inferred member function 'print(string)'.
+// We see that u has type U, so X holds.
+// X requires t.print(s) to be valid, where t has type U (substituted for T).
+// By looking at the CallExpr we find the signature of print().
+//
+// While we tend to know in advance which kind of members (access via . -> ::)
+// we want, it's simpler just to gather them all and post-filter.
+//
+// FIXME: some of this machinery could be used for non-concept type-parms too,
+// enabling completion for type parameters based on other uses of that param.
+//
+// FIXME: there are other cases where a type can be constrained by a concept,
+// e.g. inside `if constexpr(ConceptSpecializationExpr) { ... }`
+class ConceptInfo {
+public:
+ // Describes a likely member of a type, inferred by concept
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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 wrote: > > kadircet wrote: > > > as T is dependent i suppose NNS should always be an identifier, but would > > > be nice to spell it out explicitly with a comment. > > It doesn't need to be an identifier - it returns null and addType handles > > null. > i thought NNS->getAsIdentifier had an assertion instead of returning null. > > out of curiosity, when can this be non identifier though ? In general (non-dependent code) NNS is normally a decl rather than an identifier. But here in particular it can certainly be a typespec: that's the T::foo::bar case mentioned in the comment, I think the NNS stores a TemplateSpecializationType for foo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73649/new/ https://reviews.llvm.org/D73649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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 wrote: > kadircet wrote: > > as T is dependent i suppose NNS should always be an identifier, but would > > be nice to spell it out explicitly with a comment. > It doesn't need to be an identifier - it returns null and addType handles > null. i thought NNS->getAsIdentifier had an assertion instead of returning null. out of curiosity, when can this be non identifier though ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73649/new/ https://reviews.llvm.org/D73649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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())
kadircet wrote:
> nit s/concepts::Req../auto
I actually don't think the type is obvious here... before I got closely
familiar with the concepts AST, I assumed these were expressions.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4910
+ // Do a full traversal so we get `foo` from `typename T::foo::bar`.
+ QualType AssertedType = TR->getType()->getType();
+ ValidVisitor(this, T).TraverseType(AssertedType);
kadircet wrote:
> TR->getType might fail if there was a substitution failure. check for it
> before ?
>
> if(TR->isSubstitutionFailure()) continue;
substitution failures aren't dependent, so we already bailed out in that case.
Added a comment.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4984
+if (Q && isApprox(Q->getAsType(), T))
+ addType(NNS->getAsIdentifier());
+ }
kadircet wrote:
> as T is dependent i suppose NNS should always be an identifier, but would be
> nice to spell it out explicitly with a comment.
It doesn't need to be an identifier - it returns null and addType handles null.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5007
+ if (/*Inserted*/ R.second ||
+ std::make_tuple(M.Operator, M.ArgTypes.hasValue(),
+ M.ResultType != nullptr) >
kadircet wrote:
> so we'll give up result in case of (syntax might be wrong):
> ```
> ... concept C requires { T::foo; { t.foo() }-> bar }
> ```
>
> assuming we first traversed t.foo and then T::foo ? I would rather move
> operator to the end.
Done. This won't ever happen, though.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5077
+ // variables associated with DC (as returned by getTemplatedEntity()).
+ static ::SmallVector
+ constraintsForTemplatedEntity(DeclContext *DC) {
kadircet wrote:
> s/::SmallVector/llvm::SmallVector
What the heck?
(preferred spelling seems to be SmallVector here)
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5169
+ } else if (BaseType->isObjCObjectPointerType() ||
+ BaseType->isTemplateTypeParmType())
/*Do nothing*/;
kadircet wrote:
> nit:
> ```
> // ObjcPointers(properties) and TTPT(concepts) are handled below, bail out
> for the resst.
> else if (!objcPointer && ! TTPT) return false;
> ```
I actually find the 3 cases much harder to spot here, and there's no nice place
to put the comment.
Added more braces and extended the comment, WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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:
clang/include/clang/Sema/Scope.h
clang/lib/Sema/CodeCompleteConsumer.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/test/CodeCompletion/concepts.cpp
Index: clang/test/CodeCompletion/concepts.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/concepts.cpp
@@ -0,0 +1,59 @@
+template concept convertible_to = true;
+template concept same_as = true;
+template concept integral = true;
+
+template
+concept W = requires(A a, B b) {
+ { b.www } noexcept -> integral;
+};
+
+template concept X = requires(T t) {
+ t.xxx(42);
+ typename T::xxx_t;
+ T::xyz::member;
+};
+
+template
+concept Y = requires(T t, U u) { t.yyy(u); };
+
+template
+concept Z = requires(T t) {
+ { t.zzz() } -> same_as;
+ requires W;
+};
+
+// Concept constraints in all three slots require X, Y, Z, and ad-hoc stuff.
+template
+requires Y && requires(T *t) { { t->aaa() } -> convertible_to; }
+void foo(T t) requires Z || requires(T ) { t.bbb(); t->bb(); } {
+ t.x;
+ t->x;
+ T::x;
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:29:5 %s \
+ // RUN: | FileCheck %s -check-prefix=DOT -implicit-check-not=xxx_t
+ // DOT: Pattern : [#convertible_to#]aaa()
+ // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
+ // DOT: Pattern : bbb()
+ // DOT: Pattern : [#integral#]www
+ // DOT: Pattern : xxx(<#int#>)
+ // FIXME: it would be nice to have int instead of U here.
+ // DOT: Pattern : yyy(<#U#>)
+ // DOT: Pattern : [#int#]zzz()
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:30:6 %s \
+ // RUN: | FileCheck %s -check-prefix=ARROW -implicit-check-not=xxx_t
+ // ARROW: Pattern : [#convertible_to#]aaa() (requires fix-it: {{.*}} to ".")
+ // ARROW: Pattern : bb()
+ // ARROW: Pattern : bbb() (requires fix-it
+ // ARROW: Pattern : [#integral#]www (requires fix-it
+ // ARROW: Pattern : xxx(<#int#>) (requires fix-it
+ // ARROW: Pattern : yyy(<#U#>) (requires fix-it
+ // ARROW: Pattern : [#int#]zzz() (requires fix-it
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:31:6 %s \
+ // RUN: | FileCheck %s -check-prefix=COLONS -implicit-check-not=yyy
+ // COLONS: Pattern : xxx_t
+ // COLONS: Pattern : xyz
+}
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9,6 +9,7 @@
// This file defines the code-completion semantic actions.
//
//===--===//
+#include "clang/AST/ASTConcept.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
@@ -16,8 +17,11 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprConcepts.h"
#include "clang/AST/ExprObjC.h"
+#include "clang/AST/NestedNameSpecifier.h"
#include "clang/AST/QualTypeNames.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/Type.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/Specifiers.h"
@@ -4746,6 +4750,369 @@
return nullptr;
}
+namespace {
+// Collects completion-relevant information about a concept-constrainted type T.
+// In particular, examines the constraint expressions to find members of T.
+//
+// The design is very simple: we walk down each constraint looking for
+// expressions of the form T.foo().
+// If we're extra lucky, the return type is specified.
+// We don't do any clever handling of && or || in constraint expressions, we
+// take members from both branches.
+//
+// For example, given:
+// template concept X = requires (T t, string& s) { t.print(s); };
+// template void foo(U u) { u.^ }
+// We want to suggest the inferred member function 'print(string)'.
+// We see that u has type U, so X holds.
+// X requires t.print(s) to be valid, where t has type U (substituted for T).
+// By looking at the CallExpr we find the signature of print().
+//
+// While we tend to know in advance which kind of members (access via . -> ::)
+// we want, it's simpler just to gather them all and post-filter.
+//
+// FIXME: some of this machinery could be used for non-concept type-parms too,
+// enabling completion for type parameters based on other uses of that param.
+//
+// FIXME: there are other cases where a type can be constrained by a concept,
+// e.g. inside `if constexpr(ConceptSpecializationExpr) { ... }`
+class ConceptInfo {
+public:
+ // Describes a likely member of a type, inferred by concept constraints.
+ // Offered as a code completion for
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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 the member as `const` ?
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())
nit s/concepts::Req../auto
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4910
+ // Do a full traversal so we get `foo` from `typename T::foo::bar`.
+ QualType AssertedType = TR->getType()->getType();
+ ValidVisitor(this, T).TraverseType(AssertedType);
TR->getType might fail if there was a substitution failure. check for it before
?
if(TR->isSubstitutionFailure()) continue;
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4984
+if (Q && isApprox(Q->getAsType(), T))
+ addType(NNS->getAsIdentifier());
+ }
as T is dependent i suppose NNS should always be an identifier, but would be
nice to spell it out explicitly with a comment.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5007
+ if (/*Inserted*/ R.second ||
+ std::make_tuple(M.Operator, M.ArgTypes.hasValue(),
+ M.ResultType != nullptr) >
so we'll give up result in case of (syntax might be wrong):
```
... concept C requires { T::foo; { t.foo() }-> bar }
```
assuming we first traversed t.foo and then T::foo ? I would rather move
operator to the end.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5077
+ // variables associated with DC (as returned by getTemplatedEntity()).
+ static ::SmallVector
+ constraintsForTemplatedEntity(DeclContext *DC) {
s/::SmallVector/llvm::SmallVector
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5094
+
+ static QualType extractExactType(const TypeConstraint ) {
+// Assume a same_as return type constraint is std::same_as or
equivalent.
maybe rename this to `deduceType`?
Also some comments explaining that this is a `beautify heuristic` might be nice.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5169
+ } else if (BaseType->isObjCObjectPointerType() ||
+ BaseType->isTemplateTypeParmType())
/*Do nothing*/;
nit:
```
// ObjcPointers(properties) and TTPT(concepts) are handled below, bail out for
the resst.
else if (!objcPointer && ! TTPT) return false;
```
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5831
+ NestedNameSpecifier *NNS = SS.getScopeRep();
+ if (SS.isNotEmpty() && SS.isValid() && !NNS->isDependent()) {
+if (Ctx == nullptr || RequireCompleteDeclContext(SS, Ctx))
SS.isNotEmpty is same as NNS!=nullptr, maybe replace with that to make it more
clear ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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/ https://reviews.llvm.org/D73649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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:
clang/include/clang/Sema/Scope.h
clang/lib/Sema/CodeCompleteConsumer.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/test/CodeCompletion/concepts.cpp
Index: clang/test/CodeCompletion/concepts.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/concepts.cpp
@@ -0,0 +1,59 @@
+template concept convertible_to = true;
+template concept same_as = true;
+template concept integral = true;
+
+template
+concept W = requires(A a, B b) {
+ { b.www } noexcept -> integral;
+};
+
+template concept X = requires(T t) {
+ t.xxx(42);
+ typename T::xxx_t;
+ T::xyz::member;
+};
+
+template
+concept Y = requires(T t, U u) { t.yyy(u); };
+
+template
+concept Z = requires(T t) {
+ { t.zzz() } -> same_as;
+ requires W;
+};
+
+// Concept constraints in all three slots require X, Y, Z, and ad-hoc stuff.
+template
+requires Y && requires(T *t) { { t->aaa() } -> convertible_to; }
+void foo(T t) requires Z || requires(T ) { t.bbb(); t->bb(); } {
+ t.x;
+ t->x;
+ T::x;
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:29:5 %s \
+ // RUN: | FileCheck %s -check-prefix=DOT -implicit-check-not=xxx_t
+ // DOT: Pattern : [#convertible_to#]aaa()
+ // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
+ // DOT: Pattern : bbb()
+ // DOT: Pattern : [#integral#]www
+ // DOT: Pattern : xxx(<#int#>)
+ // FIXME: it would be nice to have int instead of U here.
+ // DOT: Pattern : yyy(<#U#>)
+ // DOT: Pattern : [#int#]zzz()
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:30:6 %s \
+ // RUN: | FileCheck %s -check-prefix=ARROW -implicit-check-not=xxx_t
+ // ARROW: Pattern : [#convertible_to#]aaa() (requires fix-it: {{.*}} to ".")
+ // ARROW: Pattern : bb()
+ // ARROW: Pattern : bbb() (requires fix-it
+ // ARROW: Pattern : [#integral#]www (requires fix-it
+ // ARROW: Pattern : xxx(<#int#>) (requires fix-it
+ // ARROW: Pattern : yyy(<#U#>) (requires fix-it
+ // ARROW: Pattern : [#int#]zzz() (requires fix-it
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:31:6 %s \
+ // RUN: | FileCheck %s -check-prefix=COLONS -implicit-check-not=yyy
+ // COLONS: Pattern : xxx_t
+ // COLONS: Pattern : xyz
+}
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9,6 +9,7 @@
// This file defines the code-completion semantic actions.
//
//===--===//
+#include "clang/AST/ASTConcept.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
@@ -16,8 +17,11 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprConcepts.h"
#include "clang/AST/ExprObjC.h"
+#include "clang/AST/NestedNameSpecifier.h"
#include "clang/AST/QualTypeNames.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/Type.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/Specifiers.h"
@@ -4746,6 +4750,365 @@
return nullptr;
}
+namespace {
+// Collects completion-relevant information about a concept-constrainted type T.
+// In particular, examines the constraint expressions to find members of T.
+//
+// The design is very simple: we walk down each constraint looking for
+// expressions of the form T.foo().
+// If we're extra lucky, the return type is specified.
+// We don't do any clever handling of && or || in constraint expressions, we
+// take members from both branches.
+//
+// For example, given:
+// template concept X = requires (T t, string& s) { t.print(s); };
+// template void foo(U u) { u.^ }
+// We want to suggest the inferred member function 'print(string)'.
+// We see that u has type U, so X holds.
+// X requires t.print(s) to be valid, where t has type U (substituted for T).
+// By looking at the CallExpr we find the signature of print().
+//
+// While we tend to know in advance which kind of members (access via . -> ::)
+// we want, it's simpler just to gather them all and post-filter.
+//
+// FIXME: some of this machinery could be used for non-concept type-parms too,
+// enabling completion for type parameters based on other uses of that param.
+//
+// FIXME: there are other cases where a type can be constrained by a concept,
+// e.g. inside `if constexpr(ConceptSpecializationExpr) { ... }`
+class ConceptInfo {
+public:
+ // Describes a likely member of a type, inferred by concept constraints.
+ // Offered as a code completion for T. T-> and T:: contexts.
+
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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 wrote: > So `Colons` is more info than `Arrow` which is more info than `Dot`? Is there > some intuition behind that? no, it's basically arbitrary. I don't think collisions between these are likely and worth dealing with carefully, but making the choice consistently seems worthwhile to me, in case someone ever has to debug it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73649/new/ https://reviews.llvm.org/D73649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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 nice if the test exercised this case.
> > Oops, this was actually broken because VisitNestedNameSpecifier doesn't
> > seem to be a thing :-(
> > Fixed to use TraverseNestedNameSpecifierLoc and added a test.
> > Oops, this was actually broken because VisitNestedNameSpecifier doesn't
> > seem to be a thing :-(
>
> (I suspected this, but the heavy macro usage in `RecursiveASTVisitor.h` made
> me second-guess myself and think I was just overlooking a place that defines
> `VisitNestedNameSpecifier`. I figured adding a test wouldn't hurt even if I'm
> mistaken and the code works. :-))
(Tangentially related, but a C++ editor feature I sometimes wish existed was to
show a (semantically-colored and navigation-feature-enabled)
//post-preprocessing// view of a source file. (But not edit that view! That
would be madness.) Something to add to the backlog, perhaps.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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 wrote:
> nridge wrote:
> > "given that the expression E is valid"?
> E comes from concept constraints, we actually know that E is not only valid
> but its value is exactly `true`.
>
> In particular, knowing that E is *valid* doesn't tell us anything at all
> about T if E is a ConceptSpecializationExpr like `Integral`, as we'd get
> from a `requires Integral` clause or an `Integral auto foo` parameter.
> (Note that `Integral` is a valid expression with the value
> `false`)
You're totally right on this one, my bad! (When I wrote this, I imagined
`believe()` being called on the expression inside an `ExprRequirement`, but
that's not the case.)
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4901
+if (!Req->isDependent())
+ continue; // Can't tell us anything about T.
+if (auto *TR = llvm::dyn_cast(Req)) {
sammccall wrote:
> nridge wrote:
> > Are we sure a dependent requirement can't tell us anything about `T`?
> >
> > For example, a constraint with a dependent return type requirement might
> > still give us a useful member name and arguments. Even a call expression
> > with dependent arguments could give us a useful member name.
> >
> > Or am I misunderstanding what `Requirement::isDependent()` signifies?
> I think you're reading this backwards, a *non-dependent* requirement can't
> tell us anything about T, because it doesn't depend on T!
>
> This isn't terribly important except that it takes care of a few cases at
> once (e.g. all requirements below must have an expr rather than an error,
> because constraints with an error aren't dependent)
Indeed, I was reading this backwards. Makes sense now!
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4972
+
+// In T::foo::bar, `foo` must be a type.
+bool VisitNestedNameSpecifier(NestedNameSpecifier *NNS) {
sammccall wrote:
> nridge wrote:
> > It would be nice if the test exercised this case.
> Oops, this was actually broken because VisitNestedNameSpecifier doesn't seem
> to be a thing :-(
> Fixed to use TraverseNestedNameSpecifierLoc and added a test.
> Oops, this was actually broken because VisitNestedNameSpecifier doesn't seem
> to be a thing :-(
(I suspected this, but the heavy macro usage in `RecursiveASTVisitor.h` made me
second-guess myself and think I was just overlooking a place that defines
`VisitNestedNameSpecifier`. I figured adding a test wouldn't hurt even if I'm
mistaken and the code works. :-))
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5006
+ if (/*Inserted*/ R.second ||
+ std::make_tuple(M.Operator, M.ArgTypes.hasValue(),
+ M.ResultType != nullptr) >
So `Colons` is more info than `Arrow` which is more info than `Dot`? Is there
some intuition behind that?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4772
+// FIXME: it some of this machinery could be used for non-concept tparms too,
+// enabling completion for type parameters based on other uses of that param.
+class ConceptInfo {
nridge wrote:
> Very interesting idea :)
>
> A hypothetical "infer constraints based on usage" code action could share the
> same infrastructure as well.
Yup. There'd need to be some differences - this code uses Scope which is
available during parsing but not after, a code action would need to use AST
walking, which is available after parsing but not during :-)
This is pretty hypothetical/far out/needs prototyping, so I wouldn't try to
solve any of the layering/API problems now.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4839
+ // that T is attached to in order to gather the relevant constraints.
+ ConceptInfo(const TemplateTypeParmType , Scope *S) {
+auto *TemplatedEntity = getTemplatedEntity(BaseType.getDecl(), S);
nridge wrote:
> Minor observation: based on the current usage of this class, we know at
> construction time which `AccessOperator` we want. We could potentially pass
> that in and use it to do less work in the constructor (filtering members
> earlier). However, it's not clear that this is worth it, we'd only be
> avoiding a small amount of work.
Yeah, I don't think this is worth the complexity, but added a comment to the
class.
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) {
nridge wrote:
> "given that the expression E is valid"?
E comes from concept constraints, we actually know that E is not only valid but
its value is exactly `true`.
In particular, knowing that E is *valid* doesn't tell us anything at all about
T if E is a ConceptSpecializationExpr like `Integral`, as we'd get from a
`requires Integral` clause or an `Integral auto foo` parameter. (Note that
`Integral` is a valid expression with the value `false`)
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4860
+ return;
+if (auto *CSE = llvm::dyn_cast(E)) {
+ // If the concept is
njames93 wrote:
> nridge wrote:
> > clang-tidy gives me an `'auto *CSE' can be declared as 'const auto *CSE'`
> > here, and several similar diagnostics below.
> >
> > Not sure if that's something we want to obey, or alter our configuration to
> > silence it.
> That warning will go away next time the bot updates the version of clang tidy
> it uses. It was decided to reduce the constraints on diagnosing that check
> inside llvm but that hasn't reached the pre merge bot.
Indeed, I'm no longer seeing these locally. Thanks!
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4901
+if (!Req->isDependent())
+ continue; // Can't tell us anything about T.
+if (auto *TR = llvm::dyn_cast(Req)) {
nridge wrote:
> Are we sure a dependent requirement can't tell us anything about `T`?
>
> For example, a constraint with a dependent return type requirement might
> still give us a useful member name and arguments. Even a call expression with
> dependent arguments could give us a useful member name.
>
> Or am I misunderstanding what `Requirement::isDependent()` signifies?
I think you're reading this backwards, a *non-dependent* requirement can't tell
us anything about T, because it doesn't depend on T!
This isn't terribly important except that it takes care of a few cases at once
(e.g. all requirements below must have an expr rather than an error, because
constraints with an error aren't dependent)
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4972
+
+// In T::foo::bar, `foo` must be a type.
+bool VisitNestedNameSpecifier(NestedNameSpecifier *NNS) {
nridge wrote:
> It would be nice if the test exercised this case.
Oops, this was actually broken because VisitNestedNameSpecifier doesn't seem to
be a thing :-(
Fixed to use TraverseNestedNameSpecifierLoc and added a test.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5004
+return;
+ auto = Outer->Results[Name.getAsIdentifierInfo()];
+ Result.Name = Name.getAsIdentifierInfo();
nridge wrote:
> What if there is already a result for this name?
Right, it's doing some arbitrary clobber/merge thing :-(
Fixed: now if we get a conflict we pick either the new or the old one, where
more info is better.
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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:
clang/include/clang/Sema/Scope.h
clang/lib/Sema/CodeCompleteConsumer.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/test/CodeCompletion/concepts.cpp
Index: clang/test/CodeCompletion/concepts.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/concepts.cpp
@@ -0,0 +1,59 @@
+template concept convertible_to = true;
+template concept same_as = true;
+template concept integral = true;
+
+template
+concept W = requires(A a, B b) {
+ { b.www } noexcept -> integral;
+};
+
+template concept X = requires(T t) {
+ t.xxx(42);
+ typename T::xxx_t;
+ T::xyz::member;
+};
+
+template
+concept Y = requires(T t, U u) { t.yyy(u); };
+
+template
+concept Z = requires(T t) {
+ { t.zzz() } -> same_as;
+ requires W;
+};
+
+// Concept constraints in all three slots require X, Y, Z, and ad-hoc stuff.
+template
+requires Y && requires(T *t) { { t->aaa() } -> convertible_to; }
+void foo(T t) requires Z || requires(T ) { t.bbb(); t->bb(); } {
+ t.x;
+ t->x;
+ T::x;
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:29:5 %s \
+ // RUN: | FileCheck %s -check-prefix=DOT -implicit-check-not=xxx_t
+ // DOT: Pattern : [#convertible_to#]aaa()
+ // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
+ // DOT: Pattern : bbb()
+ // DOT: Pattern : [#integral#]www
+ // DOT: Pattern : xxx(<#int#>)
+ // FIXME: it would be nice to have int instead of U here.
+ // DOT: Pattern : yyy(<#U#>)
+ // DOT: Pattern : [#int#]zzz()
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:30:6 %s \
+ // RUN: | FileCheck %s -check-prefix=ARROW -implicit-check-not=xxx_t
+ // ARROW: Pattern : [#convertible_to#]aaa() (requires fix-it: {{.*}} to ".")
+ // ARROW: Pattern : bb()
+ // ARROW: Pattern : bbb() (requires fix-it
+ // ARROW: Pattern : [#integral#]www (requires fix-it
+ // ARROW: Pattern : xxx(<#int#>) (requires fix-it
+ // ARROW: Pattern : yyy(<#U#>) (requires fix-it
+ // ARROW: Pattern : [#int#]zzz() (requires fix-it
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:31:6 %s \
+ // RUN: | FileCheck %s -check-prefix=COLONS -implicit-check-not=yyy
+ // COLONS: Pattern : xxx_t
+ // COLONS: Pattern : xyz
+}
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9,6 +9,7 @@
// This file defines the code-completion semantic actions.
//
//===--===//
+#include "clang/AST/ASTConcept.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
@@ -16,8 +17,11 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprConcepts.h"
#include "clang/AST/ExprObjC.h"
+#include "clang/AST/NestedNameSpecifier.h"
#include "clang/AST/QualTypeNames.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/Type.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/Specifiers.h"
@@ -4746,6 +4750,364 @@
return nullptr;
}
+namespace {
+// Collects completion-relevant information about a concept-constrainted type T.
+// In particular, examines the constraint expressions to find members of T.
+//
+// The design is very simple: we walk down each constraint looking for
+// expressions of the form T.foo().
+// If we're extra lucky, the return type is specified.
+// We don't do any clever handling of && or || in constraint expressions, we
+// take members from both branches.
+//
+// For example, given:
+// template concept X = requires (T t, string& s) { t.print(s); };
+// template void foo(U u) { u.^ }
+// We want to suggest the inferred member function 'print(string)'.
+// We see that u has type U, so X holds.
+// X requires t.print(s) to be valid, where t has type U (substituted for T).
+// By looking at the CallExpr we find the signature of print().
+//
+// While we tend to know in advance which kind of members (access via . -> ::)
+// we want, it's simpler just to gather them all and post-filter.
+//
+// FIXME: some of this machinery could be used for non-concept type-parms too,
+// enabling completion for type parameters based on other uses of that param.
+//
+// FIXME: there are other cases where a type can be constrained by a concept,
+// e.g. inside `if constexpr(ConceptSpecializationExpr) { ... }`
+class ConceptInfo {
+public:
+ // Describes a likely member of a type, inferred by concept constraints.
+ // Offered as a code completion for
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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'`
> here, and several similar diagnostics below.
>
> Not sure if that's something we want to obey, or alter our configuration to
> silence it.
That warning will go away next time the bot updates the version of clang tidy
it uses. It was decided to reduce the constraints on diagnosing that check
inside llvm but that hasn't reached the pre merge bot.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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: clang/lib/Sema/SemaCodeComplete.cpp:4771
+//
+// FIXME: it some of this machinery could be used for non-concept tparms too,
+// enabling completion for type parameters based on other uses of that param.
nit: stray word "it"
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4772
+// FIXME: it some of this machinery could be used for non-concept tparms too,
+// enabling completion for type parameters based on other uses of that param.
+class ConceptInfo {
Very interesting idea :)
A hypothetical "infer constraints based on usage" code action could share the
same infrastructure as well.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4839
+ // that T is attached to in order to gather the relevant constraints.
+ ConceptInfo(const TemplateTypeParmType , Scope *S) {
+auto *TemplatedEntity = getTemplatedEntity(BaseType.getDecl(), S);
Minor observation: based on the current usage of this class, we know at
construction time which `AccessOperator` we want. We could potentially pass
that in and use it to do less work in the constructor (filtering members
earlier). However, it's not clear that this is worth it, we'd only be avoiding
a small amount of work.
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) {
"given that the expression E is valid"?
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4860
+ return;
+if (auto *CSE = llvm::dyn_cast(E)) {
+ // If the concept is
clang-tidy gives me an `'auto *CSE' can be declared as 'const auto *CSE'` here,
and several similar diagnostics below.
Not sure if that's something we want to obey, or alter our configuration to
silence it.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4875
+ for (const auto : CSE->getTemplateArguments()) {
+if (Index > Params->size())
+ break; // Won't happen in valid code.
`>=` ?
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4878
+if (isApprox(Arg, T)) {
+ auto *TTPD = dyn_cast(Params->getParam(Index));
+ if (!TTPD)
We're pretty inconsistent about qualifying `dyn_cast` vs. not in this function.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4901
+if (!Req->isDependent())
+ continue; // Can't tell us anything about T.
+if (auto *TR = llvm::dyn_cast(Req)) {
Are we sure a dependent requirement can't tell us anything about `T`?
For example, a constraint with a dependent return type requirement might still
give us a useful member name and arguments. Even a call expression with
dependent arguments could give us a useful member name.
Or am I misunderstanding what `Requirement::isDependent()` signifies?
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4972
+
+// In T::foo::bar, `foo` must be a type.
+bool VisitNestedNameSpecifier(NestedNameSpecifier *NNS) {
It would be nice if the test exercised this case.
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5004
+return;
+ auto = Outer->Results[Name.getAsIdentifierInfo()];
+ Result.Name = Name.getAsIdentifierInfo();
What if there is already a result for this name?
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5044
+ if (S->isTemplateParamScope() && S->isDeclScope(D))
+return Inner->getEntity();
+ Inner = S;
Do we need to null-check `Inner` here?
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5466
- NestedNameSpecifier *NNS = SS.getScopeRep();
- if (!Results.empty() && NNS->isDependent())
Results.AddResult("template");
Is the removal of the `!Results.empty()` condition fixing a pre-existing bug?
(i.e. it looks like it's not possible for `Results` to be nonempty at this
stage?)
Comment at: clang/test/CodeCompletion/concepts.cpp:34
+ // RUN: | FileCheck %s -check-prefix=DOT -implicit-check-not=xxx_t
+ // DOT: Pattern : [#convertible_to#]aaa()
+ // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
sammccall wrote:
> nridge wrote:
> > Doesn't the presence of the `x` mean we should only get results that start
> > with `x`?
> >
> > (Or, if "column 5" actually means we're completing right after the dot, why
> > is the `x` present in the testcase at all -- just so that the
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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 is just a fix to the -code-complete-at testing facility: it wasn't
printing fixits for RK_Pattern completions)
Comment at: clang/test/CodeCompletion/concepts.cpp:34
+ // RUN: | FileCheck %s -check-prefix=DOT -implicit-check-not=xxx_t
+ // DOT: Pattern : [#convertible_to#]aaa()
+ // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
nridge wrote:
> Doesn't the presence of the `x` mean we should only get results that start
> with `x`?
>
> (Or, if "column 5" actually means we're completing right after the dot, why
> is the `x` present in the testcase at all -- just so that the line is
> syntactically well formed?)
Yeah we're completing before the x.
And in fact it doesn't matter, since clang's internal completion engine doesn't
filter the results using the incomplete identifier (which is what lets clangd
apply its own fuzzy-matching rules).
I think this is well-enough understood around the CodeCompletion tests and I
tend to worry about incomplete lines confusing the parser (I don't want to
repeat this function decl 3 times!), but I can try to drop these if you think
it makes a big difference.
Comment at: clang/test/CodeCompletion/concepts.cpp:35
+ // DOT: Pattern : [#convertible_to#]aaa()
+ // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
+ // DOT: Pattern : bbb()
nridge wrote:
> Should we be taking completions from just one branch of a logical-or in a
> requirement?
>
> To simplify the scenario a bit:
>
> ```
> template
> requires (requires(T t) { t.foo(); } || requires(T t) { t.bar(); })
> void f(T t) {
> t.^
> }
> ```
>
> Do we want to be offering both `foo()` and `bar()` as completions here?
> Logically, it seems we only ought to offer completions from expressions that
> appear in _both_ branches of the logical-or (so, if `t.foo()` appeared as a
> requirement in both branches, we could offer `foo()`).
Strictly speaking yes, a function is only guaranteed available if it's in both
branches.
In practice I think this behavior would be no more useful (probably less
useful), and obviously more complicated to implement (as we have to track
result sets for subexpressions to intersect them).
AIUI the language has no way to force you to only use properties guaranteed by
the constraints. So it's perfectly plausible to write something like (forgive
suspicious syntax)
```template requires Dumpable || Printable
void output(const T ) {
if constexpr (Dumpable)
val.dump();
else
val.print();
}```
Or even cases where the same expression is valid in all cases but we lose track
of that somehow (e.g. T is either a dumb pointer or a smart pointer to a
constrained type, and our analysis fails on the smart pointer case).
Basically I think we're desperate enough for information in these scenarios
that we'll accept a little inaccuracy :-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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 -implicit-check-not=xxx_t
+ // DOT: Pattern : [#convertible_to#]aaa()
+ // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
Doesn't the presence of the `x` mean we should only get results that start with
`x`?
(Or, if "column 5" actually means we're completing right after the dot, why is
the `x` present in the testcase at all -- just so that the line is
syntactically well formed?)
Comment at: clang/test/CodeCompletion/concepts.cpp:35
+ // DOT: Pattern : [#convertible_to#]aaa()
+ // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
+ // DOT: Pattern : bbb()
Should we be taking completions from just one branch of a logical-or in a
requirement?
To simplify the scenario a bit:
```
template
requires (requires(T t) { t.foo(); } || requires(T t) { t.bar(); })
void f(T t) {
t.^
}
```
Do we want to be offering both `foo()` and `bar()` as completions here?
Logically, it seems we only ought to offer completions from expressions that
appear in _both_ branches of the logical-or (so, if `t.foo()` appeared as a
requirement in both branches, we could offer `foo()`).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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
.
0 of them are added as review comments below (why?
).
{icon times-circle color=red} 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 .//
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73649/new/
https://reviews.llvm.org/D73649
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.
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 types are recognized and offered after ::
- variable/function members are recognized and offered after the correct
dot/arrow/colon trigger
- member functions are recognized (anything directly called). parameter types
are presumed to be the argument types. parameters are unnamed.
- result types are available when a requirement has a type constraint. These
are printed as constraints, except same_as which prints as T.
Not in this patch:
- support for merging/overloading when two locations describe the same member.
The last one wins, for any given name. This is probably important...
- support for nested template members (T::x)
- support for completing members of (instantiations of) template template
parameters
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73649
Files:
clang/include/clang/Sema/Scope.h
clang/lib/Sema/CodeCompleteConsumer.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/test/CodeCompletion/concepts.cpp
Index: clang/test/CodeCompletion/concepts.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/concepts.cpp
@@ -0,0 +1,58 @@
+template concept convertible_to = true;
+template concept same_as = true;
+template concept integral = true;
+
+template
+concept W = requires(A a, B b) {
+ { b.www } noexcept -> integral;
+};
+
+template concept X = requires(T t) {
+ t.xxx(42);
+ typename T::xxx_t;
+};
+
+template
+concept Y = requires(T t, U u) { t.yyy(u); };
+
+template
+concept Z = requires(T t) {
+ { t.zzz() } -> same_as;
+ requires W;
+};
+
+// Concept constraints in all three slots require X, Y, Z, and ad-hoc stuff.
+template
+requires Y && requires(T *t) { { t->aaa() } -> convertible_to; }
+void foo(T t) requires Z || requires(T ) { t.bbb(); t->bb(); } {
+ t.x;
+ t->x;
+ T::x;
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:28:5 %s \
+ // RUN: | FileCheck %s -check-prefix=DOT -implicit-check-not=xxx_t
+ // DOT: Pattern : [#convertible_to#]aaa()
+ // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
+ // DOT: Pattern : bbb()
+ // DOT: Pattern : [#integral#]www
+ // DOT: Pattern : xxx(<#int#>)
+ // FIXME: it would be nice to have int instead of U here.
+ // DOT: Pattern : yyy(<#U#>)
+ // DOT: Pattern : [#int#]zzz()
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:29:6 %s \
+ // RUN: | FileCheck %s -check-prefix=ARROW -implicit-check-not=xxx_t
+ // ARROW: Pattern : [#convertible_to#]aaa() (requires fix-it: {{.*}} to ".")
+ // ARROW: Pattern : bb()
+ // ARROW: Pattern : bbb() (requires fix-it
+ // ARROW: Pattern : [#integral#]www (requires fix-it
+ // ARROW: Pattern : xxx(<#int#>) (requires fix-it
+ // ARROW: Pattern : yyy(<#U#>) (requires fix-it
+ // ARROW: Pattern : [#int#]zzz() (requires fix-it
+
+ // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:30:6 %s \
+ // RUN: | FileCheck %s -check-prefix=COLONS -implicit-check-not=yyy
+ // COLONS: COMPLETION: template
+ // COLONS: Pattern : xxx_t
+}
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9,6 +9,7 @@
// This file defines the code-completion semantic actions.
//
//===--===//
+#include "clang/AST/ASTConcept.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
@@ -16,8 +17,11 @@
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprConcepts.h"
#include "clang/AST/ExprObjC.h"
+#include "clang/AST/NestedNameSpecifier.h"
#include "clang/AST/QualTypeNames.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/Type.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/Specifiers.h"
@@ -4746,6 +4750,342 @@
return nullptr;
}
+namespace {
+// Collects completion-relevant information about a concept-constrainted type T.
+// In particular, examines the constraint expressions to find members of T.
+//
+// The design is very simple: we walk down each constraint looking for
+// expressions of the form T.foo().
+// If we're extra lucky, the return type is specified.
+// We don't do any clever handling of && or || in constraint expressions, we
+// take members from both branches.
+//
+// For example, given:
+// template concept X = requires (T t, string& s) { t.print(s); };
+// template void foo(U u) { u.^ }
+// We want to suggest the inferred member function 'print(string)'.
