sammccall marked an inline comment as done. sammccall added inline comments.
================ Comment at: clang/lib/Parse/ParseInit.cpp:163 + DesignatorCompletionInfo DesignatorCompletion) { + if (!getPreprocessor().isCodeCompletionEnabled()) + DesignatorCompletion.PreferredBaseType = QualType(); // skip field lookup ---------------- kadircet wrote: > it might be nice to make the whole preferredtypebuilder a no-op on > construction when codecompletion is disabled. but that can be an adventure > for another day. Agree, this felt awkward. There's already a laziness mechanism (the function-arg stuff) but it doesn't quite fit for this case. ================ Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4803 + if (CTSD->getTemplateSpecializationKind() == TSK_Undeclared) + RD = CTSD->getSpecializedTemplate()->getTemplatedDecl(); + } ---------------- kadircet wrote: > i think this means we are going to offer completion for uninstantiated > specializations using the primary template now (through > `Sema::CodeCompleteMemberReferenceExpr`), which i am not sure is possible, > but should be an improvement in either case. > > but not having any tests (especially failing) makes me a little anxious. I think this case is vacuous for CodeCompleteMemberExpr, like you say. Before calling CodeCompleteMemberExpr, we call ActOnStartCXXMemberReference. If it's a non-dependent record type, then it calls RequireCompleteType, which will try to instantiate the template. If this fails, then we bail out and never invoke code completion. So I think there's no change in behavior for member completion, and thus can't produce a test for it. The reason for moving the fallback case into this function is that instead of 2 callsites where 1 needed the fallback, we now have 3 callsites where 2 need it. So it seems less awkward to factor this way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96058/new/ https://reviews.llvm.org/D96058 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits