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<const Expr *, 1> + constraintsForTemplatedEntity(DeclContext *DC) { ---------------- s/::SmallVector/llvm::SmallVector ================ Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5094 + + static QualType extractExactType(const TypeConstraint &T) { + // Assume a same_as<T> 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