sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2474
+TEST(CompletionTest, Lambda) {
+ clangd::CodeCompleteOptions Opts = {};
----------------
testing this in clangd is nice to have, but the canonical test for stuff in
sema code completion should be a lit test under clang/test/CodeCompletion IIRC
================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2476
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
----------------
specializing opts shouldn't be needed for this test
================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2486
+ EXPECT_EQ(Results.Completions.size(), 1u);
+ const auto A = Results.Completions.front();
+ EXPECT_EQ(A.Name, "Lambda");
----------------
auto&
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3330
+namespace {
+const CXXMethodDecl *extractLambdaCallOperator(const NamedDecl *ND) {
----------------
llvm style is to make the function static instead (we don't follow this
particlularly closely in clangd, but we should here)
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3331
+namespace {
+const CXXMethodDecl *extractLambdaCallOperator(const NamedDecl *ND) {
+ const auto *VD = dyn_cast<VarDecl>(ND);
----------------
in view of the functor (std::function etc) case, should this be
extractFunctorCallOperator()?
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3335
+ return nullptr;
+ const auto *LambdaType = VD->getType().getTypePtr();
+ if (const auto *SugaredType = dyn_cast<AutoType>(LambdaType))
----------------
VD->getType()->getAsCXXRecordDecl() should be all you need.
getAs() and friends desugar as needed.
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3342
+ const auto *RecordDecl = LambdaType->getAsCXXRecordDecl();
+ // FIXME: Don't ignore generic lambdas.
+ return (RecordDecl && RecordDecl->isLambda() &&
----------------
can we fix that instead of leaving a FIXME? should only be a couple of lines I
think
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3342
+ const auto *RecordDecl = LambdaType->getAsCXXRecordDecl();
+ // FIXME: Don't ignore generic lambdas.
+ return (RecordDecl && RecordDecl->isLambda() &&
----------------
sammccall wrote:
> can we fix that instead of leaving a FIXME? should only be a couple of lines
> I think
also FIXME: support other functor types like std::function?
looking at the implementation of getLambdaCallOperatorHelper it looks pretty
easy
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3374
- AddResultTypeChunk(Ctx, Policy, ND, CCContext.getBaseType(), Result);
-
- if (const auto *Function = dyn_cast<FunctionDecl>(ND)) {
- AddQualifierToCompletionString(Result, Qualifier, QualifierIsInformative,
- Ctx, Policy);
+ auto handleFunctionlike = [&](const FunctionDecl *Function,
+ bool AddQualifier = true) {
----------------
can you give this a clearer name?
Like AddFunctionTypeAndResult?
I also think making it void would be clearer, rather than mixing
adding/building.
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3378
+ if (AddQualifier)
+ AddQualifierToCompletionString(Result, Qualifier, QualifierIsInformative,
+ Ctx, Policy);
----------------
why do you want to avoid adding the qualifier in lambda case?
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3396
+
+ AddResultTypeChunk(Ctx, Policy, ND, CCContext.getBaseType(), Result);
+
----------------
above the return statement to below - why?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70445/new/
https://reviews.llvm.org/D70445
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
