[CodeComplete] Member completion for concept-constrained types.

[sammccall]

Summary

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<T> 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<int>)
• support for completing members of (instantiations of) template template parameters

[merge_guards_bot]

Unit tests: pass. 62282 tests passed, 0 failed and 827 were skipped.

clang-tidy: fail. clang-tidy found 0 errors and 7 warnings. 0 of them are added as review comments below (why?).

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 or enable it for your project.

[nridge]

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.

clang/test/CodeCompletion/concepts.cpp

35

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?)

36

Should we be taking completions from just one branch of a logical-or in a requirement?

To simplify the scenario a bit:

template <typename T>
  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()).

[sammccall]

clang/lib/Sema/CodeCompleteConsumer.cpp

592

(This is just a fix to the -code-complete-at testing facility: it wasn't printing fixits for RK_Pattern completions)

clang/test/CodeCompletion/concepts.cpp

35

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.

36

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 <typename T> requires Dumpable<T> || Printable<T>
void output(const T &val) {
  if constexpr (Dumpable<T>)
    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 :-)

[nridge]

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.

clang/lib/Sema/SemaCodeComplete.cpp

4771

nit: stray word "it"

4772

Very interesting idea :)

A hypothetical "infer constraints based on usage" code action could share the same infrastructure as well.

4839

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.

4856

"given that the expression E is valid"?

4860

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.

4875

>= ?

4878

We're pretty inconsistent about qualifying dyn_cast vs. not in this function.

4901

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?

4972

It would be nice if the test exercised this case.

5004

What if there is already a result for this name?

5044

Do we need to null-check Inner here?

5466

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?)

clang/test/CodeCompletion/concepts.cpp

35

Thanks. No change needed here, just wanted to clarify my own understanding.

36

I think in the mentioned example, the ideal behaviour would be to offer dump() as a completion only in the then-branch of the if constexpr, and offer print() only in the else-branch.

But that's quite a bit more involved to implement, and I agree that for now, it's more useful to offer both completions in the entire function than to offer neither.

Maybe just add a "TODO" comment (or even just a "to potentially explore in the future" comment) about this? (Could be in the code rather than the test.)

[njames93]

clang/lib/Sema/SemaCodeComplete.cpp

4860

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.

[sammccall]

Address review comments

[sammccall]

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.

clang/lib/Sema/SemaCodeComplete.cpp

4772

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.

4839

Yeah, I don't think this is worth the complexity, but added a comment to the class.

4856

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<T>, as we'd get from a requires Integral<T> clause or an Integral auto foo parameter. (Note that Integral<std::string> is a valid expression with the value false)

4860

Indeed, I'm no longer seeing these locally. Thanks!

4901

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)

4972

Oops, this was actually broken because VisitNestedNameSpecifier doesn't seem to be a thing :-(
Fixed to use TraverseNestedNameSpecifierLoc and added a test.

5004

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.

5044

Done. It's that or assert that the initial S is never the declaring scope of D, which it never seems to be.
But these are just passed through from elsewhere in sema and I'm really leery of relying on any undocumented invariants.

5466

Oops, yeah. Not the right time to fix it. Added a FIXME instead.

clang/test/CodeCompletion/concepts.cpp

36

Added this as a comment on ConceptInfo.

[nridge]

clang/lib/Sema/SemaCodeComplete.cpp

4856

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.)

4901

Indeed, I was reading this backwards. Makes sense now!

4972

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. :-))

5006

So Colons is more info than Arrow which is more info than Dot? Is there some intuition behind that?

[nridge]

clang/lib/Sema/SemaCodeComplete.cpp

4972

(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.)

[sammccall]

comment and tweak order of AccessOperator

[sammccall]

clang/lib/Sema/SemaCodeComplete.cpp

5006

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.

[nridge]

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

[sammccall]

@kadircet I realized this change was gathering dust, and you've touched SemaCodeComplete in recent memory... any interest?

[kadircet]

clang/include/clang/Sema/Scope.h

323

also mark the member as const ?

clang/lib/Sema/SemaCodeComplete.cpp

4905

nit s/concepts::Req../auto

4910

TR->getType might fail if there was a substitution failure. check for it before ?

if(TR->isSubstitutionFailure()) continue;

4984

as T is dependent i suppose NNS should always be an identifier, but would be nice to spell it out explicitly with a comment.

5007

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.

5077

s/::SmallVector/llvm::SmallVector

5094

maybe rename this to deduceType?

Also some comments explaining that this is a beautify heuristic might be nice.

5168

nit:

// ObjcPointers(properties) and TTPT(concepts) are handled below, bail out for the resst.
else if (!objcPointer && ! TTPT) return false;

5830

SS.isNotEmpty is same as NNS!=nullptr, maybe replace with that to make it more clear ?

[sammccall]

address review comments

[sammccall]

Thanks!

clang/lib/Sema/SemaCodeComplete.cpp

4905

I actually don't think the type is obvious here... before I got closely familiar with the concepts AST, I assumed these were expressions.

4910

substitution failures aren't dependent, so we already bailed out in that case. Added a comment.

4984

It doesn't need to be an identifier - it returns null and addType handles null.

5007

Done. This won't ever happen, though.

5077

What the heck?

(preferred spelling seems to be SmallVector here)

5168

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?

[kadircet]

LGTM. thanks!

clang/lib/Sema/SemaCodeComplete.cpp

4984

i thought NNS->getAsIdentifier had an assertion instead of returning null.

out of curiosity, when can this be non identifier though ?

[sammccall]

clang/lib/Sema/SemaCodeComplete.cpp

4984

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<X>::bar case mentioned in the comment, I think the NNS stores a TemplateSpecializationType for foo<X>.

clang/include/clang/Sema/Scope.h

public:
   }
 
   unsigned getMSCurManglingNumber() const {
     return MSCurManglingNumber;
   }
 
   /// isDeclScope - Return true if this is the scope that the specified decl is
   /// declared in.
-  bool isDeclScope(Decl *D) {
-    return DeclsInScope.count(D) != 0;
-  }
+  bool isDeclScope(const Decl *D) { return DeclsInScope.count(D) != 0; }
 
   DeclContext *getEntity() const { return Entity; }
   void setEntity(DeclContext *E) { Entity = E; }
 
   bool hasErrorOccurred() const { return ErrorTrap.hasErrorOccurred(); }
 
   bool hasUnrecoverableErrorOccurred() const {
     return ErrorTrap.hasUnrecoverableErrorOccurred();

clang/lib/Sema/CodeCompleteConsumer.cpp

case CodeCompletionResult::RK_Declaration:
       }
       if (CodeCompletionString *CCS = Results[I].CreateCodeCompletionString(
               SemaRef, Context, getAllocator(), CCTUInfo,
               includeBriefComments())) {
         OS << " : " << CCS->getAsString();
         if (const char *BriefComment = CCS->getBriefComment())
           OS << " : " << BriefComment;
       }
-      for (const FixItHint &FixIt : Results[I].FixIts) {
-        const SourceLocation BLoc = FixIt.RemoveRange.getBegin();
-        const SourceLocation ELoc = FixIt.RemoveRange.getEnd();
+      break;
+
+    case CodeCompletionResult::RK_Keyword:
+      OS << Results[I].Keyword;
+      break;
+
+    case CodeCompletionResult::RK_Macro:
+      OS << Results[I].Macro->getName();
+      if (CodeCompletionString *CCS = Results[I].CreateCodeCompletionString(
+              SemaRef, Context, getAllocator(), CCTUInfo,
+              includeBriefComments())) {
+        OS << " : " << CCS->getAsString();
+      }
+      break;
+
+    case CodeCompletionResult::RK_Pattern:
+      OS << "Pattern : " << Results[I].Pattern->getAsString();
+      break;
+    }
 
-        SourceManager &SM = SemaRef.SourceMgr;
-        std::pair<FileID, unsigned> BInfo = SM.getDecomposedLoc(BLoc);
-        std::pair<FileID, unsigned> EInfo = SM.getDecomposedLoc(ELoc);
-        // Adjust for token ranges.
-        if (FixIt.RemoveRange.isTokenRange())
-          EInfo.second += Lexer::MeasureTokenLength(ELoc, SM, SemaRef.LangOpts);
 
-        OS << " (requires fix-it:"
-           << " {" << SM.getLineNumber(BInfo.first, BInfo.second) << ':'
-           << SM.getColumnNumber(BInfo.first, BInfo.second) << '-'
-           << SM.getLineNumber(EInfo.first, EInfo.second) << ':'
-           << SM.getColumnNumber(EInfo.first, EInfo.second) << "}"
-           << " to \"" << FixIt.CodeToInsert << "\")";
-      }
-      OS << '\n';
-      break;
-
-    case CodeCompletionResult::RK_Keyword:
-      OS << Results[I].Keyword << '\n';
-      break;
-
-    case CodeCompletionResult::RK_Macro:
-      OS << Results[I].Macro->getName();
-      if (CodeCompletionString *CCS = Results[I].CreateCodeCompletionString(
-              SemaRef, Context, getAllocator(), CCTUInfo,
-              includeBriefComments())) {
-        OS << " : " << CCS->getAsString();
-      }
-      OS << '\n';
-      break;
-
-    case CodeCompletionResult::RK_Pattern:
-      OS << "Pattern : " << Results[I].Pattern->getAsString() << '\n';
-      break;
-    }
   }
 }
 
 // This function is used solely to preserve the former presentation of overloads
 // by "clang -cc1 -code-completion-at", since CodeCompletionString::getAsString
 // needs to be improved for printing the newer and more detailed overload
 // chunks.
 static std::string getOverloadAsString(const CodeCompletionString &CCS) {

clang/lib/Sema/SemaCodeComplete.cpp

 //===---------------- SemaCodeComplete.cpp - Code Completion ----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  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"
 #include "clang/AST/DeclObjC.h"
 #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"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "clang/Sema/Designator.h"
if (const auto *TD = dyn_cast_or_null<ClassTemplateDecl>(
             TST->getTemplateName().getAsTemplateDecl())) {
       return TD->getTemplatedDecl();
     }
   }
 
   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 <class T> concept X = requires (T t, string& s) { t.print(s); };
+//   template <X U> void foo(U u) { u.^ }
+// We want to suggest the inferred member function 'print(string)'.
+// We see that u has type U, so X<U> holds.
+// X<U> 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.
+  struct Member {
+    // Always non-null: we only handle members with ordinary identifier names.
+    const IdentifierInfo *Name = nullptr;
+    // Set for functions we've seen called.
+    // We don't have the declared parameter types, only the actual types of
+    // arguments we've seen. These are still valuable, as it's hard to render
+    // a useful function completion with neither parameter types nor names!
+    llvm::Optional<llvm::SmallVector<QualType, 1>> ArgTypes;
+    // Whether this is accessed as T.member, T->member, or T::member.
+    enum AccessOperator {
+      Dot,
+      Arrow,
+      Colons,
+    } Operator = Dot;
+    // What's known about the type of a variable or return type of a function.
+    const TypeConstraint *ResultType = nullptr;
+    // FIXME: also track:
+    //   - kind of entity (function/variable/type), to expose structured results
+    //   - template args kinds/types, as a proxy for template params
+
+    // For now we simply return these results as "pattern" strings.
+    CodeCompletionString *render(Sema &S, CodeCompletionAllocator &Alloc,
+                                 CodeCompletionTUInfo &Info) const {
+      CodeCompletionBuilder B(Alloc, Info);
+      // Result type
+      if (ResultType) {
+        std::string AsString;
+        {
+          llvm::raw_string_ostream OS(AsString);
+          QualType ExactType = extractExactType(*ResultType);
+          if (!ExactType.isNull())
+            ExactType.print(OS, getCompletionPrintingPolicy(S));
+          else
+            ResultType->print(OS, getCompletionPrintingPolicy(S));
+        }
+        B.AddResultTypeChunk(Alloc.CopyString(AsString));
+      }
+      // Member name
+      B.AddTypedTextChunk(Alloc.CopyString(Name->getName()));
+      // Function argument list
+      if (ArgTypes) {
+        B.AddChunk(clang::CodeCompletionString::CK_LeftParen);
+        bool First = true;
+        for (QualType Arg : *ArgTypes) {
+          if (First)
+            First = false;
+          else {
+            B.AddChunk(clang::CodeCompletionString::CK_Comma);
+            B.AddChunk(clang::CodeCompletionString::CK_HorizontalSpace);
+          }
+          B.AddPlaceholderChunk(Alloc.CopyString(
+              Arg.getAsString(getCompletionPrintingPolicy(S))));
+        }
+        B.AddChunk(clang::CodeCompletionString::CK_RightParen);
+      }
+      return B.TakeString();
+    }
+  };
+
+  // BaseType is the type parameter T to infer members from.
+  // T must be accessible within S, as we use it to find the template entity
+  // that T is attached to in order to gather the relevant constraints.
+  ConceptInfo(const TemplateTypeParmType &BaseType, Scope *S) {
+    auto *TemplatedEntity = getTemplatedEntity(BaseType.getDecl(), S);
+    for (const Expr *E : constraintsForTemplatedEntity(TemplatedEntity))
+      believe(E, &BaseType);
+  }
+
+  std::vector<Member> members() {
+    std::vector<Member> Results;
+    for (const auto &E : this->Results)
+      Results.push_back(E.second);
+    llvm::sort(Results, [](const Member &L, const Member &R) {
+      return L.Name->getName() < R.Name->getName();
+    });
+    return Results;
+  }
+
+private:
+  // Infer members of T, given that the expression E (dependent on T) is true.
+  void believe(const Expr *E, const TemplateTypeParmType *T) {
+    if (!E || !T)
+      return;
+    if (auto *CSE = dyn_cast<ConceptSpecializationExpr>(E)) {
+      // If the concept is
+      //   template <class A, class B> concept CD = f<A, B>();
+      // And the concept specialization is
+      //   CD<int, T>
+      // Then we're substituting T for B, so we want to make f<A, B>() true
+      // by adding members to B - i.e. believe(f<A, B>(), B);
+      //
+      // For simplicity:
+      // - we don't attempt to substitute int for A
+      // - when T is used in other ways (like CD<T*>) we ignore it
+      ConceptDecl *CD = CSE->getNamedConcept();
+      TemplateParameterList *Params = CD->getTemplateParameters();
+      unsigned Index = 0;
+      for (const auto &Arg : CSE->getTemplateArguments()) {
+        if (Index >= Params->size())
+          break; // Won't happen in valid code.
+        if (isApprox(Arg, T)) {
+          auto *TTPD = dyn_cast<TemplateTypeParmDecl>(Params->getParam(Index));
+          if (!TTPD)
+            continue;
+          // T was used as an argument, and bound to the parameter TT.
+          auto *TT = cast<TemplateTypeParmType>(TTPD->getTypeForDecl());
+          // So now we know the constraint as a function of TT is true.
+          believe(CD->getConstraintExpr(), TT);
+          // (concepts themselves have no associated constraints to require)
+        }
+
+        ++Index;
+      }
+    } else if (auto *BO = dyn_cast<BinaryOperator>(E)) {
+      // For A && B, we can infer members from both branches.
+      // For A || B, the union is still more useful than the intersection.
+      if (BO->getOpcode() == BO_LAnd || BO->getOpcode() == BO_LOr) {
+        believe(BO->getLHS(), T);
+        believe(BO->getRHS(), T);
+      }
+    } else if (auto *RE = dyn_cast<RequiresExpr>(E)) {
+      // A requires(){...} lets us infer members from each requirement.
+      for (const concepts::Requirement *Req : RE->getRequirements()) {
+        if (!Req->isDependent())
+          continue; // Can't tell us anything about T.
+        if (auto *TR = dyn_cast<concepts::TypeRequirement>(Req)) {
+          // Do a full traversal so we get `foo` from `typename T::foo::bar`.
+          QualType AssertedType = TR->getType()->getType();
+          ValidVisitor(this, T).TraverseType(AssertedType);
+        } else if (auto *ER = dyn_cast<concepts::ExprRequirement>(Req)) {
+          ValidVisitor Visitor(this, T);
+          // If we have a type constraint on the value of the expression,
+          // AND the whole outer expression describes a member, then we'll
+          // be able to use the constraint to provide the return type.
+          if (ER->getReturnTypeRequirement().isTypeConstraint()) {
+            Visitor.OuterType =
+                ER->getReturnTypeRequirement().getTypeConstraint();
+            Visitor.OuterExpr = ER->getExpr();
+          }
+          Visitor.TraverseStmt(ER->getExpr());
+        } else if (auto *NR = dyn_cast<concepts::NestedRequirement>(Req)) {
+          believe(NR->getConstraintExpr(), T);
+        }
+      }
+    }
+  }
+
+  // This visitor infers members of T based on traversing expressions/types
+  // that involve T. It is invoked with code known to be valid for T.
+  class ValidVisitor : public RecursiveASTVisitor<ValidVisitor> {
+    ConceptInfo *Outer;
+    const TemplateTypeParmType *T;
+
+    CallExpr *Caller = nullptr;
+    Expr *Callee = nullptr;
+
+  public:
+    // If set, OuterExpr is constrained by OuterType.
+    Expr *OuterExpr = nullptr;
+    const TypeConstraint *OuterType = nullptr;
+
+    ValidVisitor(ConceptInfo *Outer, const TemplateTypeParmType *T)
+        : Outer(Outer), T(T) {
+      assert(T);
+    }
+
+    // In T.foo or T->foo, `foo` is a member function/variable.
+    bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+      const Type *Base = E->getBaseType().getTypePtr();
+      bool IsArrow = E->isArrow();
+      if (Base->isPointerType() && IsArrow) {
+        IsArrow = false;
+        Base = Base->getPointeeType().getTypePtr();
+      }
+      if (isApprox(Base, T))
+        addValue(E, E->getMember(), IsArrow ? Member::Arrow : Member::Dot);
+      return true;
+    }
+
+    // In T::foo, `foo` is a static member function/variable.
+    bool VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) {
+      if (E->getQualifier() && isApprox(E->getQualifier()->getAsType(), T))
+        addValue(E, E->getDeclName(), Member::Colons);
+      return true;
+    }
+
+    // In T::typename foo, `foo` is a type.
+    bool VisitDependentNameType(DependentNameType *DNT) {
+      const auto *Q = DNT->getQualifier();
+      if (Q && isApprox(Q->getAsType(), T))
+        addType(DNT->getIdentifier());
+      return true;
+    }
+
+    // In T::foo::bar, `foo` must be a type.
+    // VisitNNS() doesn't exist, and TraverseNNS isn't always called :-(
+    bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSL) {
+      if (NNSL) {
+        NestedNameSpecifier *NNS = NNSL.getNestedNameSpecifier();
+        const auto *Q = NNS->getPrefix();
+        if (Q && isApprox(Q->getAsType(), T))
+          addType(NNS->getAsIdentifier());
+      }
+      // FIXME: also handle T::foo<X>::bar
+      return RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(NNSL);
+    }
+
+    // FIXME also handle T::foo<X>
+
+    // Track the innermost caller/callee relationship so we can tell if a
+    // nested expr is being called as a function.
+    bool VisitCallExpr(CallExpr *CE) {
+      Caller = CE;
+      Callee = CE->getCallee();
+      return true;
+    }
+
+  private:
+    void addResult(Member &&M) {
+      auto R = Outer->Results.try_emplace(M.Name);
+      Member &O = R.first->second;
+      // Overwrite existing if the new member has more info.
+      if (/*Inserted*/ R.second ||
+          std::make_tuple(M.Operator, M.ArgTypes.hasValue(),
+                          M.ResultType != nullptr) >
+              std::make_tuple(O.Operator, O.ArgTypes.hasValue(),
+                              O.ResultType != nullptr))
+        O = std::move(M);
+    }
+
+    void addType(const IdentifierInfo *Name) {
+      if (!Name)
+        return;
+      Member M;
+      M.Name = Name;
+      M.Operator = Member::Colons;
+      addResult(std::move(M));
+    }
+
+    void addValue(Expr *E, DeclarationName Name,
+                  Member::AccessOperator Operator) {
+      if (!Name.isIdentifier())
+        return;
+      Member Result;
+      Result.Name = Name.getAsIdentifierInfo();
+      Result.Operator = Operator;
+      // If this is the callee of an immediately-enclosing CallExpr, then
+      // treat it as a method, otherwise it's a variable.
+      if (Caller != nullptr && Callee == E) {
+        Result.ArgTypes.emplace();
+        for (const auto *Arg : Caller->arguments())
+          Result.ArgTypes->push_back(Arg->getType());
+        if (Caller == OuterExpr) {
+          Result.ResultType = OuterType;
+        }
+      } else {
+        if (E == OuterExpr)
+          Result.ResultType = OuterType;
+      }
+      addResult(std::move(Result));
+    }
+  };
+
+  static bool isApprox(const TemplateArgument &Arg, const Type *T) {
+    return Arg.getKind() == TemplateArgument::Type &&
+           isApprox(Arg.getAsType().getTypePtr(), T);
+  }
+
+  static bool isApprox(const Type *T1, const Type *T2) {
+    return T1 && T2 &&
+           T1->getCanonicalTypeUnqualified() ==
+               T2->getCanonicalTypeUnqualified();
+  }
+
+  // Returns the DeclContext immediately enclosed by the template parameter
+  // scope. For primary templates, this is the templated (e.g.) CXXRecordDecl.
+  // For specializations, this is e.g. ClassTemplatePartialSpecializationDecl.
+  static DeclContext *getTemplatedEntity(const TemplateTypeParmDecl *D,
+                                         Scope *S) {
+    if (D == nullptr)
+      return nullptr;
+    Scope *Inner = nullptr;
+    while (S) {
+      if (S->isTemplateParamScope() && S->isDeclScope(D))
+        return Inner ? Inner->getEntity() : nullptr;
+      Inner = S;
+      S = S->getParent();
+    }
+    return nullptr;
+  }
+
+  // Gets all the type constraint expressions that might apply to the type
+  // variables associated with DC (as returned by getTemplatedEntity()).
+  static ::SmallVector<const Expr *, 1>
+  constraintsForTemplatedEntity(DeclContext *DC) {
+    llvm::SmallVector<const Expr *, 1> Result;
+    if (DC == nullptr)
+      return Result;
+    // Primary templates can have constraints.
+    if (const auto *TD = cast<Decl>(DC)->getDescribedTemplate())
+      TD->getAssociatedConstraints(Result);
+    // Partial specializations may have constraints.
+    if (const auto *CTPSD =
+            dyn_cast<ClassTemplatePartialSpecializationDecl>(DC))
+      CTPSD->getAssociatedConstraints(Result);
+    if (const auto *VTPSD = dyn_cast<VarTemplatePartialSpecializationDecl>(DC))
+      VTPSD->getAssociatedConstraints(Result);
+    return Result;
+  }
+
+  static QualType extractExactType(const TypeConstraint &T) {
+    // Assume a same_as<T> return type constraint is std::same_as or equivalent.
+    // In this case the return type is T.
+    DeclarationName DN = T.getNamedConcept()->getDeclName();
+    if (DN.isIdentifier() && DN.getAsIdentifierInfo()->isStr("same_as"))
+      if (const auto *Args = T.getTemplateArgsAsWritten())
+        if (Args->getNumTemplateArgs() == 1) {
+          const auto &Arg = Args->arguments().front().getArgument();
+          if (Arg.getKind() == TemplateArgument::Type)
+            return Arg.getAsType();
+        }
+    return {};
+  }
+
+  llvm::DenseMap<const IdentifierInfo *, Member> Results;
+};
+} // namespace
+
 void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
                                            Expr *OtherOpBase,
                                            SourceLocation OpLoc, bool IsArrow,
                                            bool IsBaseExprStatement,
                                            QualType PreferredType) {
   if (!Base || !CodeCompleter)
     return;
 
auto DoCompletion = [&](Expr *Base, bool IsArrow,
     if (BaseType.isNull())
       return false;
     ExprValueKind BaseKind = Base->getValueKind();
 
     if (IsArrow) {
       if (const PointerType *Ptr = BaseType->getAs<PointerType>()) {
         BaseType = Ptr->getPointeeType();
         BaseKind = VK_LValue;
-      } else if (BaseType->isObjCObjectPointerType())
+      } else if (BaseType->isObjCObjectPointerType() ||
+                 BaseType->isTemplateTypeParmType())
         /*Do nothing*/;
       else
         return false;
     }
 
     if (RecordDecl *RD = getAsRecordDecl(BaseType)) {
       AddRecordMembersCompletionResults(*this, Results, S, BaseType, BaseKind,
                                         RD, std::move(AccessOpFixIt));
+    } else if (const auto *TTPT =
+                   dyn_cast<TemplateTypeParmType>(BaseType.getTypePtr())) {
+      auto Operator =
+          IsArrow ? ConceptInfo::Member::Arrow : ConceptInfo::Member::Dot;
+      for (const auto &R : ConceptInfo(*TTPT, S).members()) {
+        if (R.Operator != Operator)
+          continue;
+        CodeCompletionResult Result(
+            R.render(*this, CodeCompleter->getAllocator(),
+                     CodeCompleter->getCodeCompletionTUInfo()));
+        if (AccessOpFixIt)
+          Result.FixIts.push_back(*AccessOpFixIt);
+        Results.AddResult(std::move(Result));
+      }
     } else if (!IsArrow && BaseType->isObjCObjectPointerType()) {
       // Objective-C property reference.
       AddedPropertiesSet AddedProperties;
 
       if (const ObjCObjectPointerType *ObjCPtr =
               BaseType->getAsObjCInterfacePointerType()) {
         // Add property results based on our interface.
         assert(ObjCPtr && "Non-NULL pointer guaranteed above!");
if (SS.isInvalid()) {
     }
     HandleCodeCompleteResults(this, CodeCompleter,
                               DummyResults.getCompletionContext(), nullptr, 0);
     return;
   }
   // Always pretend to enter a context to ensure that a dependent type
   // resolves to a dependent record.
   DeclContext *Ctx = computeDeclContext(SS, /*EnteringContext=*/true);
-  if (!Ctx)
-    return;
 
   // Try to instantiate any non-dependent declaration contexts before
-  // we look in them.
-  if (!isDependentScopeSpecifier(SS) && RequireCompleteDeclContext(SS, Ctx))
+  // we look in them. Bail out if we fail.
+  NestedNameSpecifier *NNS = SS.getScopeRep();
+  if (SS.isNotEmpty() && SS.isValid() && !NNS->isDependent()) {
+    if (Ctx == nullptr || RequireCompleteDeclContext(SS, Ctx))
       return;
+  }
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
                         CodeCompleter->getCodeCompletionTUInfo(), CC);
   if (!PreferredType.isNull())
     Results.setPreferredType(PreferredType);
   Results.EnterNewScope();
 
   // The "template" keyword can follow "::" in the grammar, but only
   // put it into the grammar if the nested-name-specifier is dependent.
-  NestedNameSpecifier *NNS = SS.getScopeRep();
+  // FIXME: results is always empty, this appears to be dead.
   if (!Results.empty() && NNS->isDependent())
     Results.AddResult("template");
 
+  // If the scope is a concept-constrained type parameter, infer nested
+  // members based on the constraints.
+  if (const auto *TTPT =
+          dyn_cast_or_null<TemplateTypeParmType>(NNS->getAsType())) {
+    for (const auto &R : ConceptInfo(*TTPT, S).members()) {
+      if (R.Operator != ConceptInfo::Member::Colons)
+        continue;
+      Results.AddResult(CodeCompletionResult(
+          R.render(*this, CodeCompleter->getAllocator(),
+                   CodeCompleter->getCodeCompletionTUInfo())));
+    }
+  }
+
   // Add calls to overridden virtual functions, if there are any.
   //
   // FIXME: This isn't wonderful, because we don't know whether we're actually
   // in a context that permits expressions. This is a general issue with
   // qualified-id completions.
-  if (!EnteringContext)
+  if (Ctx && !EnteringContext)
     MaybeAddOverrideCalls(*this, Ctx, Results);
   Results.ExitScope();
 
-  if (CodeCompleter->includeNamespaceLevelDecls() ||
-      (!Ctx->isNamespace() && !Ctx->isTranslationUnit())) {
+  if (Ctx &&
+      (CodeCompleter->includeNamespaceLevelDecls() || !Ctx->isFileContext())) {
     CodeCompletionDeclConsumer Consumer(Results, Ctx, BaseType);
     LookupVisibleDecls(Ctx, LookupOrdinaryName, Consumer,
                        /*IncludeGlobalScope=*/true,
                        /*IncludeDependentBases=*/true,
                        CodeCompleter->loadExternal());
   }
 
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),

clang/test/CodeCompletion/concepts.cpp

+template <typename T, typename U> concept convertible_to = true;
+template <typename T, typename U> concept same_as = true;
+template <typename T> concept integral = true;
+
+template <typename A, typename B>
+concept W = requires(A a, B b) {
+  { b.www } noexcept -> integral;
+};
+
+template <typename T> concept X = requires(T t) {
+  t.xxx(42);
+  typename T::xxx_t;
+  T::xyz::member;
+};
+
+template <typename T, typename U>
+concept Y = requires(T t, U u) { t.yyy(u); };
+
+template <typename T>
+concept Z = requires(T t) {
+  { t.zzz() } -> same_as<int>;
+  requires W<int, T>;
+};
+
+// Concept constraints in all three slots require X, Y, Z, and ad-hoc stuff.
+template <X T>
+requires Y<T, int> && requires(T *t) { { t->aaa() } -> convertible_to<double>; }
+void foo(T t) requires Z<T> || requires(T &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<double>#]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<double>#]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
+}
+