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

[Ivan Donchevskii via Phabricator via cfe-commits]

yvvan added inline comments.


================
Comment at: lib/Sema/CodeCompleteConsumer.cpp:559
+        const char *Begin =
+            SemaRef.SourceMgr.getCharacterData(FixIt.RemoveRange.getBegin());
+        const char *End =
----------------
ilya-biryukov wrote:
> 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";
> ```
yeah. sorry. it's always like that with source ranges. thanks for a hint


================
Comment at: lib/Sema/SemaCodeComplete.cpp:4109
+  if (CodeCompleter->includeFixIts()) {
+    const SourceRange OpRange(OpLoc, OpLoc.getLocWithOffset(IsArrow ? 2 : 1));
+    CompletionSucceded =
----------------
ilya-biryukov wrote:
> 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.
the problem is that I need the range, not just a location.... and I don't know how to extract it here. is there a way to get next token location here?


https://reviews.llvm.org/D41537