[PATCH] Implementation for completion in call context for C++
Francisco Lopes da Silva
francisco.mailing.lists at oblita.com
Fri Jan 16 03:37:03 PST 2015
================
Comment at: lib/Sema/SemaCodeComplete.cpp:3824-3828
@@ +3823,7 @@
+
+template <unsigned N>
+void mergeCandidatesWithResults(SmallVector<ResultCandidate, N> &Results,
+ OverloadCandidateSet &CandidateSet,
+ SourceLocation Loc,
+ Sema &S) {
+ if (!CandidateSet.empty()) {
----------------
klimek wrote:
> http://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h
> """Prefer to use SmallVectorImpl<T> as a parameter type."""
> That also removes the need for this to be templated at all iiuc.
Thanks, I missed this tip.
================
Comment at: lib/Sema/SemaCodeComplete.cpp:3892-3895
@@ +3891,6 @@
+ TemplateArgumentListInfo TemplateArgsBuffer, *TemplateArgs = nullptr;
+ if (UME->hasExplicitTemplateArgs()) {
+ UME->copyTemplateArgumentsInto(TemplateArgsBuffer);
+ TemplateArgs = &TemplateArgsBuffer;
+ }
+ SmallVector<Expr*, 12> ArgExprs(1, UME->getBase());
----------------
klimek wrote:
> I still don't understand why we need this instead of just calling getExplicitTemplateArgs when we need it...
`AddFunctionCandidates` expects `TemplateArgumentListInfo *` which is not compatible with the return of `getExplicitTemplateArgs` or `getOptionalExplicitTemplateArgs`.
================
Comment at: lib/Sema/SemaCodeComplete.cpp:3919-3920
@@ +3918,4 @@
+ } else {
+ // Lastly we check, as a possibly resolved expression, whether it can be
+ // converted to a function
+ FunctionDecl *FD = nullptr;
----------------
klimek wrote:
> Add "." in the end of a sentence. Also, thanks for adding those comments, they help me tremendously :)
OK.
================
Comment at: lib/Sema/SemaOverload.cpp:12477-12482
@@ +12476,7 @@
+
+bool clang::TooManyArguments(size_t NumParams, size_t NumArgs,
+ bool PartialOverloading) {
+ if (NumArgs > 0 && PartialOverloading)
+ return NumArgs + 1 > NumParams;
+ return NumArgs > NumParams;
+}
----------------
klimek wrote:
> 1. are we not in namespace clang yet??
> 2. This could still need a comment explaining exactly why we need the +1 - the next person coming along will wonder the same thing, so we can save them a lot of time by leaving a comprehensive explanation... :)
1. If you're just asking: no, I have declared it in namespace `clang` (since it's used in `SemaTemplateDeduction.cpp`) and must be defined inside it.
If you're asking why it's in namespace `clang` and not elsewhere: it was the best place I could infer.
2. OK.
http://reviews.llvm.org/D6880
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list