[PATCH] D41537: Optionally add code completion results for arrow instead of dot

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I have only a few nits left, otherwise looks good. Thanks for making this change! Really excited for it to get in finally!



================
Comment at: include/clang/Driver/CC1Options.td:449
+def code_completion_with_fixits : Flag<["-"], "code-completion-with-fixits">,
+  HelpText<"Include code completion results which require small fix-its .">;
 def disable_free : Flag<["-"], "disable-free">,
----------------
NIT: there's a redundant space before the closing `.`


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:786
 
+  /// \brief FixIts that *must* be applied before inserting the text for the
+  /// corresponding completion item.
----------------
NIT: remove \brief from the start of the comment. Those recently got removed from all of llvm


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:1059
 
+  /// \brief Whether to include completion items with small fix-its, e.g. change
+  /// '.' to '->' on member access, etc.
----------------
NIT: another redundant `\brief`


================
Comment at: include/clang/Sema/CodeCompleteOptions.h:42
 
+  /// Show also results after corrections (small fix-its), e.g. change '.' to
+  /// '->' on member access, etc.
----------------
NIT: Maybe `s/Show also results/Include results`?


================
Comment at: lib/Sema/SemaCodeComplete.cpp:4148
+
+  CompletionSucceded |= DoCompletion(Base, IsArrow, None);
+
----------------
NIT: maybe swap the two cases to do the non-fixit ones first? It is the most common case, so it should probably go first.


https://reviews.llvm.org/D41537