[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added inline comments.


================
Comment at: clang/lib/Sema/CodeCompleteConsumer.cpp:592
     }
+    for (const FixItHint &FixIt : 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<double>#]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<double>#]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 <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()`).
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 :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73649/new/

https://reviews.llvm.org/D73649