From 96b8b58101fe5e5532648db63bd90f5d56c5bea8 Mon Sep 17 00:00:00 2001 From: Ilya Biryukov Date: Fri, 7 Sep 2018 14:04:39 +0000 Subject: [PATCH] [CodeComplete] Clearly distinguish signature help and code completion. Summary: Code completion in clang is actually a mix of two features: - Code completion is a familiar feature. Results are exposed via the CodeCompleteConsumer::ProcessCodeCompleteResults callback. - Signature help figures out if the current expression is an argument of some function call and shows corresponding signatures if so. Results are exposed via CodeCompleteConsumer::ProcessOverloadCandidates. This patch refactors the implementation to untangle those two from each other and makes some naming tweaks to avoid confusion when reading the code. The refactoring is required for signature help fixes, see D51038. The only intended behavior change is the order of callbacks. ProcessOverloadCandidates is now called before ProcessCodeCompleteResults. Reviewers: sammccall, kadircet Reviewed By: sammccall Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D51782 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@341660 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Sema/Sema.h | 14 ++++--- lib/Parse/ParseDecl.cpp | 7 ++-- lib/Parse/ParseExpr.cpp | 10 +++-- lib/Parse/ParseExprCXX.cpp | 6 ++- lib/Parse/ParseOpenMP.cpp | 3 +- lib/Sema/SemaCodeComplete.cpp | 69 +++++++++++++++-------------------- test/CodeCompletion/call.cpp | 2 +- 7 files changed, 55 insertions(+), 56 deletions(-) diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 465e8cddb5..04ba15edb3 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -10231,6 +10231,7 @@ public: struct CodeCompleteExpressionData; void CodeCompleteExpression(Scope *S, const CodeCompleteExpressionData &Data); + void CodeCompleteExpression(Scope *S, QualType PreferredType); void CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base, Expr *OtherOpBase, SourceLocation OpLoc, bool IsArrow, bool IsBaseExprStatement); @@ -10241,11 +10242,14 @@ public: const VirtSpecifiers *VS = nullptr); void CodeCompleteBracketDeclarator(Scope *S); void CodeCompleteCase(Scope *S); - void CodeCompleteCall(Scope *S, Expr *Fn, ArrayRef Args, - SourceLocation OpenParLoc); - void CodeCompleteConstructor(Scope *S, QualType Type, SourceLocation Loc, - ArrayRef Args, - SourceLocation OpenParLoc); + /// Reports signatures for a call to CodeCompleteConsumer and returns the + /// preferred type for the current argument. Returned type can be null. + QualType ProduceCallSignatureHelp(Scope *S, Expr *Fn, ArrayRef Args, + SourceLocation OpenParLoc); + QualType ProduceConstructorSignatureHelp(Scope *S, QualType Type, + SourceLocation Loc, + ArrayRef Args, + SourceLocation OpenParLoc); void CodeCompleteInitializer(Scope *S, Decl *D); void CodeCompleteReturn(Scope *S); void CodeCompleteAfterIf(Scope *S); diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 5d780de8f5..7a3a019898 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -2302,16 +2302,17 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes( llvm::function_ref ExprListCompleter; auto ThisVarDecl = dyn_cast_or_null(ThisDecl); auto ConstructorCompleter = [&, ThisVarDecl] { - Actions.CodeCompleteConstructor( + QualType PreferredType = Actions.ProduceConstructorSignatureHelp( getCurScope(), ThisVarDecl->getType()->getCanonicalTypeInternal(), ThisDecl->getLocation(), Exprs, T.getOpenLocation()); + Actions.CodeCompleteExpression(getCurScope(), PreferredType); }; if (ThisVarDecl) { // ParseExpressionList can sometimes succeed even when ThisDecl is not // VarDecl. This is an error and it is reported in a call to // Actions.ActOnInitializerError(). However, we call - // CodeCompleteConstructor only on VarDecls, falling back to default - // completer in other cases. + // ProduceConstructorSignatureHelp only on VarDecls, falling back to + // default completer in other cases. ExprListCompleter = ConstructorCompleter; } diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index 39bf1c6398..d3bb2bbc24 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/lib/Parse/ParseExpr.cpp @@ -1650,8 +1650,9 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) { CommaLocsTy CommaLocs; if (Tok.is(tok::code_completion)) { - Actions.CodeCompleteCall(getCurScope(), LHS.get(), None, - PT.getOpenLocation()); + QualType PreferredType = Actions.ProduceCallSignatureHelp( + getCurScope(), LHS.get(), None, PT.getOpenLocation()); + Actions.CodeCompleteExpression(getCurScope(), PreferredType); cutOffParsing(); return ExprError(); } @@ -1659,8 +1660,9 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) { if (OpKind == tok::l_paren || !LHS.isInvalid()) { if (Tok.isNot(tok::r_paren)) { if (ParseExpressionList(ArgExprs, CommaLocs, [&] { - Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs, - PT.getOpenLocation()); + QualType PreferredType = Actions.ProduceCallSignatureHelp( + getCurScope(), LHS.get(), ArgExprs, PT.getOpenLocation()); + Actions.CodeCompleteExpression(getCurScope(), PreferredType); })) { (void)Actions.CorrectDelayedTyposInExpr(LHS); LHS = ExprError(); diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp index fef01a4030..deafd04506 100644 --- a/lib/Parse/ParseExprCXX.cpp +++ b/lib/Parse/ParseExprCXX.cpp @@ -1685,9 +1685,10 @@ Parser::ParseCXXTypeConstructExpression(const DeclSpec &DS) { if (Tok.isNot(tok::r_paren)) { if (ParseExpressionList(Exprs, CommaLocs, [&] { - Actions.CodeCompleteConstructor( + QualType PreferredType = Actions.ProduceConstructorSignatureHelp( getCurScope(), TypeRep.get()->getCanonicalTypeInternal(), DS.getEndLoc(), Exprs, T.getOpenLocation()); + Actions.CodeCompleteExpression(getCurScope(), PreferredType); })) { SkipUntil(tok::r_paren, StopAtSemi); return ExprError(); @@ -2819,9 +2820,10 @@ Parser::ParseCXXNewExpression(bool UseGlobal, SourceLocation Start) { if (ParseExpressionList(ConstructorArgs, CommaLocs, [&] { ParsedType TypeRep = Actions.ActOnTypeName(getCurScope(), DeclaratorInfo).get(); - Actions.CodeCompleteConstructor( + QualType PreferredType = Actions.ProduceConstructorSignatureHelp( getCurScope(), TypeRep.get()->getCanonicalTypeInternal(), DeclaratorInfo.getEndLoc(), ConstructorArgs, ConstructorLParen); + Actions.CodeCompleteExpression(getCurScope(), PreferredType); })) { SkipUntil(tok::semi, StopAtSemi | StopBeforeMatch); return ExprError(); diff --git a/lib/Parse/ParseOpenMP.cpp b/lib/Parse/ParseOpenMP.cpp index c53eae3067..b3c7e3a63d 100644 --- a/lib/Parse/ParseOpenMP.cpp +++ b/lib/Parse/ParseOpenMP.cpp @@ -418,10 +418,11 @@ void Parser::ParseOpenMPReductionInitializerForDecl(VarDecl *OmpPrivParm) { SourceLocation LParLoc = T.getOpenLocation(); if (ParseExpressionList( Exprs, CommaLocs, [this, OmpPrivParm, LParLoc, &Exprs] { - Actions.CodeCompleteConstructor( + QualType PreferredType = Actions.ProduceConstructorSignatureHelp( getCurScope(), OmpPrivParm->getType()->getCanonicalTypeInternal(), OmpPrivParm->getLocation(), Exprs, LParLoc); + Actions.CodeCompleteExpression(getCurScope(), PreferredType); })) { Actions.ActOnInitializerError(OmpPrivParm); SkipUntil(tok::r_paren, tok::annot_pragma_openmp_end, StopBeforeMatch); diff --git a/lib/Sema/SemaCodeComplete.cpp b/lib/Sema/SemaCodeComplete.cpp index 7ba94c6e0e..addddd8a84 100644 --- a/lib/Sema/SemaCodeComplete.cpp +++ b/lib/Sema/SemaCodeComplete.cpp @@ -3752,6 +3752,10 @@ void Sema::CodeCompleteExpression(Scope *S, Results.data(), Results.size()); } +void Sema::CodeCompleteExpression(Scope *S, QualType PreferredType) { + return CodeCompleteExpression(S, CodeCompleteExpressionData(PreferredType)); +} + void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) { if (E.isInvalid()) CodeCompleteOrdinaryName(S, PCC_RecoveryInFunction); @@ -4435,42 +4439,28 @@ static QualType getParamType(Sema &SemaRef, return ParamType; } -static void -CodeCompleteOverloadResults(Sema &SemaRef, Scope *S, - MutableArrayRef Candidates, - unsigned CurrentArg, SourceLocation OpenParLoc, - bool CompleteExpressionWithCurrentArg = true) { - QualType ParamType; - if (CompleteExpressionWithCurrentArg) - ParamType = getParamType(SemaRef, Candidates, CurrentArg); - - if (ParamType.isNull()) - SemaRef.CodeCompleteOrdinaryName(S, Sema::PCC_Expression); - else - SemaRef.CodeCompleteExpression(S, ParamType); - - if (!Candidates.empty()) - SemaRef.CodeCompleter->ProcessOverloadCandidates( - SemaRef, CurrentArg, Candidates.data(), Candidates.size(), OpenParLoc); +static QualType +ProduceSignatureHelp(Sema &SemaRef, Scope *S, + MutableArrayRef Candidates, + unsigned CurrentArg, SourceLocation OpenParLoc) { + if (Candidates.empty()) + return QualType(); + SemaRef.CodeCompleter->ProcessOverloadCandidates( + SemaRef, CurrentArg, Candidates.data(), Candidates.size(), OpenParLoc); + return getParamType(SemaRef, Candidates, CurrentArg); } -void Sema::CodeCompleteCall(Scope *S, Expr *Fn, ArrayRef Args, - SourceLocation OpenParLoc) { +QualType Sema::ProduceCallSignatureHelp(Scope *S, Expr *Fn, + ArrayRef Args, + SourceLocation OpenParLoc) { if (!CodeCompleter) - return; - - // When we're code-completing for a call, we fall back to ordinary - // name code-completion whenever we can't produce specific - // results. We may want to revisit this strategy in the future, - // e.g., by merging the two kinds of results. + return QualType(); // FIXME: Provide support for variadic template functions. - // Ignore type-dependent call expressions entirely. if (!Fn || Fn->isTypeDependent() || anyNullArguments(Args) || Expr::hasAnyTypeDependentArguments(Args)) { - CodeCompleteOrdinaryName(S, PCC_Expression); - return; + return QualType(); } // Build an overload candidate set based on the functions we find. @@ -4551,25 +4541,24 @@ void Sema::CodeCompleteCall(Scope *S, Expr *Fn, ArrayRef Args, Results.push_back(ResultCandidate(FT)); } } - mergeCandidatesWithResults(*this, Results, CandidateSet, Loc); - CodeCompleteOverloadResults(*this, S, Results, Args.size(), OpenParLoc, - !CandidateSet.empty()); + QualType ParamType = + ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc); + return !CandidateSet.empty() ? ParamType : QualType(); } -void Sema::CodeCompleteConstructor(Scope *S, QualType Type, SourceLocation Loc, - ArrayRef Args, - SourceLocation OpenParLoc) { +QualType Sema::ProduceConstructorSignatureHelp(Scope *S, QualType Type, + SourceLocation Loc, + ArrayRef Args, + SourceLocation OpenParLoc) { if (!CodeCompleter) - return; + return QualType(); // A complete type is needed to lookup for constructors. CXXRecordDecl *RD = isCompleteType(Loc, Type) ? Type->getAsCXXRecordDecl() : nullptr; - if (!RD) { - CodeCompleteExpression(S, Type); - return; - } + if (!RD) + return Type; // FIXME: Provide support for member initializers. // FIXME: Provide support for variadic template constructors. @@ -4594,7 +4583,7 @@ void Sema::CodeCompleteConstructor(Scope *S, QualType Type, SourceLocation Loc, SmallVector Results; mergeCandidatesWithResults(*this, Results, CandidateSet, Loc); - CodeCompleteOverloadResults(*this, S, Results, Args.size(), OpenParLoc); + return ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc); } void Sema::CodeCompleteInitializer(Scope *S, Decl *D) { diff --git a/test/CodeCompletion/call.cpp b/test/CodeCompletion/call.cpp index 3e062109a9..c57c339cd1 100644 --- a/test/CodeCompletion/call.cpp +++ b/test/CodeCompletion/call.cpp @@ -18,10 +18,10 @@ void f(); void test() { f(Y(), 0, 0); // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:19:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s - // CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>) // CHECK-CC1: f(Y y, <#int ZZ#>) // CHECK-CC1-NEXT: f(int i, <#int j#>, int k) // CHECK-CC1-NEXT: f(float x, <#float y#>) + // CHECK-CC1: COMPLETION: Pattern : dynamic_cast<<#type#>>(<#expression#>) // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:19:13 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s // CHECK-CC2-NOT: f(Y y, int ZZ) // CHECK-CC2: f(int i, int j, <#int k#>) -- 2.40.0