[PATCH] D53654: [clang] Improve ctor initializer completions.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: lib/Sema/SemaCodeComplete.cpp:813
 
+static void AddTemplateParameterChunks(ASTContext &Context,
+                                       const PrintingPolicy &Policy,
----------------
We don't seem to need this fwd-decl anymore. Remove it?


================
Comment at: lib/Sema/SemaCodeComplete.cpp:821
+
+DeclContext::lookup_result getConstructorResults(ASTContext &Context,
+                                                 const CXXRecordDecl *Record,
----------------
There's a function that does the same thing and also does some extra lifting to make sure implicit members are properly generated -  `Sema::LookupConstructors`. Maybe use it instead?


================
Comment at: lib/Sema/SemaCodeComplete.cpp:831
+  // template args and one parameter with type's name.
+  /*
+  if (Ctors.begin() == Ctors.end()) {
----------------
Remove this code instead of commenting it out?


================
Comment at: lib/Sema/SemaCodeComplete.cpp:882
+  auto Ctors = getConstructorResults(SemaRef.Context, Record, R);
+  for (DeclContext::lookup_iterator I = Ctors.begin(), E = Ctors.end(); I != E;
+       ++I) {
----------------
Maybe use range-baseed for loop?


================
Comment at: lib/Sema/SemaCodeComplete.cpp:5123
+
+  auto generateCCS = [&](const NamedDecl *ND, const char *Name) {
+    Builder.AddTypedTextChunk(Name);
----------------
NIT: naming, use UpperCamelCase


================
Comment at: lib/Sema/SemaCodeComplete.cpp:5135
+  };
+  auto AddDefaultCtorInit = [&](const char *TypedName,
+                                const char *TypeName,
----------------
kadircet wrote:
> ZaMaZaN4iK wrote:
> > Is it good idea to capture ALL by reference? Probably will be better to capture only required things by reference
> That's the case within the rest of the code, so I did that to be consistent, and apart from that I don't think this brings any disadvantages.
`TypedName` and `TypeName` are too similar. Maybe pick names that are easier to distinguish?
`Name` and `Type` seem to be a good fit


================
Comment at: lib/Sema/SemaCodeComplete.cpp:5136
+  auto AddDefaultCtorInit = [&](const char *TypedName,
+                                const char *TypeName,
+                                const NamedDecl* ND) {
----------------
Maybe use StringRef?


================
Comment at: lib/Sema/SemaCodeComplete.cpp:5151
+  auto AddCtorsWithName = [&](const CXXRecordDecl *RD,
+                              const CodeCompletionResult R, const char *Name) {
+    auto Ctors = getConstructorResults(Context, RD, R);
----------------
`CodeCompletionResult R`  looks redundant here? Do we really need it?


================
Comment at: lib/Sema/SemaCodeComplete.cpp:5166
+    const auto *RD = Base.getType()->getAsCXXRecordDecl();
+    if (!RD)
+      return AddDefaultCtorInit(BaseName, BaseName, nullptr);
----------------
The code past that point is the same between AddBase and AddField. Maybe move it into AddCtorsWithName?


Repository:
  rC Clang

https://reviews.llvm.org/D53654