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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry, a few more things and we're good to go.



================
Comment at: include/clang/Driver/CC1Options.td:449
+def code_completion_include_fixits : Flag<["-"], "code-completion-include-fixits">,
+  HelpText<"Include code completion results which require having small fix-its applied.">;
 def disable_free : Flag<["-"], "disable-free">,
----------------
NIT: maybe simplify to "which require small fix-its"?


================
Comment at: lib/Frontend/CompilerInvocation.cpp:1541
+  Opts.CodeCompleteOpts.IncludeFixIts
+    = Args.hasArg(OPT_code_completion_include_fixits);
 
----------------
Following naming convention of other options, maybe use `OPT_code_completion_with_fixits`? (i.e. none of them start with include)


================
Comment at: lib/Sema/CodeCompleteConsumer.cpp:559
+        const char *Begin =
+            SemaRef.SourceMgr.getCharacterData(FixIt.RemoveRange.getBegin());
+        const char *End =
----------------
Unfortunately, that might not work for some ranges, see docs of `CharSourceRange`:
```
/// In the token range case, the
/// size of the last token must be measured to determine the actual end of the
/// range.
```

The `TextDiagnostic::emitParseableFixits` handles it, I suggest we do it similarly:
```
    FixItHint*I = //....;
    SourceLocation BLoc = I->RemoveRange.getBegin();
    SourceLocation ELoc = I->RemoveRange.getEnd();

    std::pair<FileID, unsigned> BInfo = SM.getDecomposedLoc(BLoc);
    std::pair<FileID, unsigned> EInfo = SM.getDecomposedLoc(ELoc);

    // Adjust for token ranges.
    if (I->RemoveRange.isTokenRange())
      EInfo.second += Lexer::MeasureTokenLength(ELoc, SM, LangOpts);

    // We specifically do not do word-wrapping or tab-expansion here,
    // because this is supposed to be easy to parse.
    PresumedLoc PLoc = SM.getPresumedLoc(BLoc);
    if (PLoc.isInvalid())
      break;

    OS << "fix-it:\"";
    OS.write_escaped(PLoc.getFilename());
    OS << "\":{" << SM.getLineNumber(BInfo.first, BInfo.second)
      << ':' << SM.getColumnNumber(BInfo.first, BInfo.second)
      << '-' << SM.getLineNumber(EInfo.first, EInfo.second)
      << ':' << SM.getColumnNumber(EInfo.first, EInfo.second)
      << "}:\"";
    OS.write_escaped(I->CodeToInsert);
    OS << "\"\n";
```


================
Comment at: lib/Sema/SemaCodeComplete.cpp:4109
+  if (CodeCompleter->includeFixIts()) {
+    const SourceRange OpRange(OpLoc, OpLoc.getLocWithOffset(IsArrow ? 2 : 1));
+    CompletionSucceded =
----------------
I'd use token ranges here to avoid assumptions about sizes of tokens, e.g. `CreateReplacemen(CharSourceRange::getTokenRange(OpLoc, OpLoc), IsArrow ? '.' : '->')`
There are complicated cases like `\` that end of the line and macros and it's definitely better to use an abstraction that hides those cases.


https://reviews.llvm.org/D41537