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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
