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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
