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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:767
+  /// When Kind == RK_Declaration and a FieldDecl has been passed as
+  /// Declaration, this will hold the identifiers name. To be used later on when
+  /// generating constructors for the variable.
----------------
NIT: does "identifiers name" refer to a field name? Maybe use "field name" directly to avoid possibly confusion.


================
Comment at: lib/Sema/SemaCodeComplete.cpp:3027
                                    Ctx, Policy);
-    AddTypedNameChunk(Ctx, Policy, ND, Result);
+    AddTypedNameChunk(Ctx, Policy, ND, Result, PreferredTypedName);
     Result.AddChunk(CodeCompletionString::CK_LeftParen);
----------------
Maybe directly set the name here whenever PreferredTypedName is set and avoid changing the AddTypedNameChunk function?
This would keep the `AddTypedNameChunk` simpler and `PreferredTypedName` would simply become an overriden name that the code completion string should use instead of the declaration name. 
I.e.
```
if (PreferredTypedName)
  Result.AddTypedTextChunk(Result.getAllocator().CopyString(PreferredTypedName));
else
  AddTypedNameChunk(Ctx, Policy, ...);
```
 


================
Comment at: lib/Sema/SemaCodeComplete.cpp:5192
+    const CXXRecordDecl *RD = Field->getType()->getAsCXXRecordDecl();
+    if (!RD) {
+      Builder.AddTypedTextChunk(Builder.getAllocator().CopyString(FieldName));
----------------
Should we also have fallbacks for bases in addition to fields? They may also have non-class types.
Could we also add a test? 
Should pop up in completions of dependent bases, e.g.
```
template <class T>
struct Y : T {};

template <class T>
struct X : std::vector<T> {}; // referring to an explicit template, however still dependent.
```



================
Comment at: lib/Sema/SemaCodeComplete.cpp:5193
+    if (!RD) {
+      Builder.AddTypedTextChunk(Builder.getAllocator().CopyString(FieldName));
+      Builder.AddChunk(CodeCompletionString::CK_LeftParen);
----------------
Maybe add a comment that this is a fallback?


================
Comment at: test/CodeCompletion/ordinary-name-cxx11.cpp:65
   // CHECK-CC1: COMPLETION: X : X
+  // CHECK-CC1-NEXT: COMPLETION: Pattern : X(<#X#>)
   // CHECK-CC1-NEXT: COMPLETION: y : [#int#]y
----------------
This completion is outside constructor initializers.
Why behavior change for these completions too?


Repository:
  rC Clang

https://reviews.llvm.org/D53654