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<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`)
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<concepts::TypeRequirement>(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
