Skip to content

Commit

Permalink
[clangd] Correctly identify the next token after the completion point (
Browse files Browse the repository at this point in the history
…#69153)

The code was previously using Lexer::findNextToken() which does not
handle being passed the completion point as input.

Fixes clangd/clangd#1785
  • Loading branch information
HighCommander4 committed Nov 12, 2023
1 parent d5a0fb3 commit cee5987
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 1 deletion.
42 changes: 41 additions & 1 deletion clang-tools-extra/clangd/CodeComplete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1491,6 +1491,46 @@ FuzzyFindRequest speculativeFuzzyFindRequestForCompletion(
return CachedReq;
}

// This function is similar to Lexer::findNextToken(), but assumes
// that the input SourceLocation is the completion point (which is
// a case findNextToken() does not handle).
std::optional<Token>
findTokenAfterCompletionPoint(SourceLocation CompletionPoint,
const SourceManager &SM,
const LangOptions &LangOpts) {
SourceLocation Loc = CompletionPoint;
if (Loc.isMacroID()) {
if (!Lexer::isAtEndOfMacroExpansion(Loc, SM, LangOpts, &Loc))
return std::nullopt;
}

// Advance to the next SourceLocation after the completion point.
// Lexer::findNextToken() would call MeasureTokenLength() here,
// which does not handle the completion point (and can't, because
// the Lexer instance it constructs internally doesn't have a
// Preprocessor and so doesn't know about the completion point).
Loc = Loc.getLocWithOffset(1);

// Break down the source location.
std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Loc);

// Try to load the file buffer.
bool InvalidTemp = false;
StringRef File = SM.getBufferData(LocInfo.first, &InvalidTemp);
if (InvalidTemp)
return std::nullopt;

const char *TokenBegin = File.data() + LocInfo.second;

// Lex from the start of the given location.
Lexer TheLexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts, File.begin(),
TokenBegin, File.end());
// Find the token.
Token Tok;
TheLexer.LexFromRawLexer(Tok);
return Tok;
}

// Runs Sema-based (AST) and Index-based completion, returns merged results.
//
// There are a few tricky considerations:
Expand Down Expand Up @@ -1589,7 +1629,7 @@ class CodeCompleteFlow {
auto Style = getFormatStyleForFile(SemaCCInput.FileName,
SemaCCInput.ParseInput.Contents,
*SemaCCInput.ParseInput.TFS);
const auto NextToken = Lexer::findNextToken(
const auto NextToken = findTokenAfterCompletionPoint(
Recorder->CCSema->getPreprocessor().getCodeCompletionLoc(),
Recorder->CCSema->getSourceManager(), Recorder->CCSema->LangOpts);
if (NextToken)
Expand Down
23 changes: 23 additions & 0 deletions clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3892,6 +3892,29 @@ TEST(CompletionTest, FunctionArgsExist) {
kind(CompletionItemKind::Function))));
}

TEST(CompletionTest, FunctionArgsExist_Issue1785) {
// This is a scenario where the implementation of our check for
// "is there a function argument list right after the cursor"
// gave a bogus result.
clangd::CodeCompleteOptions Opts;
Opts.EnableSnippets = true;
// The whitespace in this testcase is important!
std::string Code = R"cpp(
void waldo(int);
int main()
{
wal^
// ( )
}
)cpp";
EXPECT_THAT(
completions(Code, {}, Opts).Completions,
Contains(AllOf(labeled("waldo(int)"), snippetSuffix("(${1:int})"))));
}

TEST(CompletionTest, NoCrashDueToMacroOrdering) {
EXPECT_THAT(completions(R"cpp(
#define ECHO(X) X
Expand Down

0 comments on commit cee5987

Please sign in to comment.